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

kvch · 2021-10-27T13:36:42Z

What does this PR do?

This PR removes auto from the available options of setup.ilm.enabled. Previously, auto was used to ease users into using ILM in 7.x by only loading ILM if Elasticsearch supported it.

Why is it important?

With moving to 8.x, the option no longer make sense, because the supported Elasticsearch supports ILM always. Also, as we are adopting data streams, the suggested solution is to use ILM to manage the backing indices of data streams. Thus, I would not like to expose the option of managing indices with Beats without ILM. We should let users skip loading an ILM policy, but we should rely on ILM for index management.

In this PR a moderately complex part of ILM loading is removed. 🎉

Checklist

My code follows the style guidelines of this project
~~- [ ] I have commented my code, particularly in hard-to-understand areas~~
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2021-10-27T13:36:45Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

mergify · 2021-10-27T13:37:35Z

This pull request does not have a backport label. Could you fix it @kvch? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2021-10-27T13:51:28Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-10-28T08:00:50.415+0000
Duration: 106 min 38 sec
Commit: 18c749d

Test stats 🧪

Test	Results
Failed	0
Passed	54080
Skipped	5359
Total	59439

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.

ruflin · 2021-10-28T08:30:07Z

libbeat/docs/shared-ilm.asciidoc

@@ -26,7 +26,7 @@ Example configuration:

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


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

libbeat/idxmgmt/ilm/client_handler.go

libbeat/idxmgmt/ilm/ilm.go

ruflin · 2021-10-29T08:26:11Z

libbeat/idxmgmt/ilm/client_handler.go

 			return false, errOf(ErrESVersionNotSupported)
 		}
 		return false, nil
 	}

-	if !enabled && mode == ModeEnabled {
+	if !enabledILM && enabled {


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

ruflin · 2021-10-29T08:28:53Z

libbeat/idxmgmt/ilm/client_handler.go

-	}
-	return avail, probe
+func checkILMVersion(ver common.Version) (avail bool) {
+	return !ver.LessThan(esMinILMVersion)


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 🥳

So let's clean this up 👍🏼

…ue to true (elastic#28671)

…ue to true (#28671) (cherry picked from commit 39d1541)

…ue to true (#28671) (#28959) (cherry picked from commit 39d1541) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

ghost · 2021-12-06T08:14:35Z

Hi @kvch,

We have gone through this ticket and tried to locate the proposed changes. However, we are unable to find 'setup.ilm.enabled' property in beats yml files.

Could you please brief us on how to validate the above merged PR.

Thanks,
QAS

kvch · 2021-12-06T18:25:44Z

The option is called setup.ilm.enabled. You can find it in the reference configuration here: https://github.com/elastic/beats/blob/7.16/filebeat/filebeat.reference.yml#L2135
What you need to check is the default value is true and that ILM gets loaded as before.

ghost · 2021-12-07T11:16:49Z

Hi @kvch

Thanks for sharing the feedback on our queries.

We are now able to fetch the option under filebeat.reference.yml file in commented form.

Query: can you please confirm if we need to uncomment it from now while doing beats install on 8.0 and also if any other steps are required to perform so that beats data is generated into data streams and not as indices.

Screenshot:

Thanks
QAS

kvch · 2021-12-07T14:49:35Z

No need to uncomment it. It is enabled by default.

kvch added 2 commits October 27, 2021 15:24

rm auto option

51fd678

add docs

af292c4

kvch added 8.0-candidate Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Oct 27, 2021

kvch requested a review from ruflin October 27, 2021 13:36

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 27, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 27, 2021

add changelog entry

472c873

follow up change in test

18c749d

ruflin reviewed Oct 28, 2021

View reviewed changes

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

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

kvch mentioned this pull request Oct 28, 2021

Setup data streams and send events to them #28450

Merged

6 tasks

ruflin approved these changes Oct 29, 2021

View reviewed changes

kvch merged commit 39d1541 into elastic:master Oct 29, 2021

Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021

Remove auto option from setup.ilm.enabled and set the default val…

cd6b0f2

…ue to true (elastic#28671)

axw mentioned this pull request Nov 15, 2021

[8.0] Update to elastic/beats@4d7e91351f6e elastic/apm-server#6605

Merged

kvch added the backport-v8.0.0 Automated backport with mergify label Nov 15, 2021

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Nov 15, 2021

mergify bot mentioned this pull request Nov 15, 2021

[8.0](backport #28671) Remove auto option from setup.ilm.enabled and set the default value to true #28959

Merged

mergify bot pushed a commit that referenced this pull request Nov 15, 2021

Remove auto option from setup.ilm.enabled and set the default val…

75849d3

…ue to true (#28671) (cherry picked from commit 39d1541)

kvch added a commit that referenced this pull request Nov 15, 2021

Remove auto option from setup.ilm.enabled and set the default val…

2af9608

…ue to true (#28671) (#28959) (cherry picked from commit 39d1541) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

jlind23 added v8.0.0 and removed 8.0-candidate labels Nov 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

kvch commented Oct 27, 2021 •

edited

Loading

elasticmachine commented Oct 27, 2021

mergify bot commented Oct 27, 2021

elasticmachine commented Oct 27, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

ruflin Oct 28, 2021

kvch Oct 28, 2021

ruflin Oct 29, 2021

ruflin Oct 29, 2021

jlind23 Oct 29, 2021

ghost commented Dec 6, 2021

kvch commented Dec 6, 2021

ghost commented Dec 7, 2021 •

edited by ghost

Loading

kvch commented Dec 7, 2021

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

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

Conversation

kvch commented Oct 27, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Oct 27, 2021

mergify bot commented Oct 27, 2021

elasticmachine commented Oct 27, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

ruflin Oct 28, 2021

Choose a reason for hiding this comment

kvch Oct 28, 2021

Choose a reason for hiding this comment

ruflin Oct 29, 2021

Choose a reason for hiding this comment

ruflin Oct 29, 2021

Choose a reason for hiding this comment

jlind23 Oct 29, 2021

Choose a reason for hiding this comment

ghost commented Dec 6, 2021

kvch commented Dec 6, 2021

ghost commented Dec 7, 2021 • edited by ghost Loading

kvch commented Dec 7, 2021

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

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

kvch commented Oct 27, 2021 •

edited

Loading

elasticmachine commented Oct 27, 2021 •

edited by jenkins-beats-ci bot

Loading

ghost commented Dec 7, 2021 •

edited by ghost

Loading