Do not load ML jobs to 8.x from 7.x releases #27771

kvch · 2021-09-07T11:20:33Z

What does this PR do?

This PR adds a check before loading ML assets to make sure, Beats are loading those to ES 7.x. If someone tries to load ML into ES 8.x, they are told to use the Machine Learning UI in Kibana.

This PR is opened against 7.x.

Why is it important?

This work is part of the forward compatibility effort. We have to make sure users rely on the better solution for loading ML in Kibana, instead of the deprecated setup --machine-learning subcommand.

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~~ it will come in a follow up PR
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2021-09-07T11:20:46Z

Pinging @elastic/agent (Team:Agent)

elasticmachine · 2021-09-07T12:33:59Z

💚 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-06T13:14:46.681+0000
Duration: 152 min 59 sec
Commit: f5347df

Test stats 🧪

Test	Results
Failed	0
Passed	54235
Skipped	5292
Total	59527

💚 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.

kvch · 2021-09-08T14:12:36Z

Failing tests are unrelated.

ruflin · 2021-09-09T08:08:17Z

filebeat/fileset/modules.go

@@ -392,6 +392,11 @@ func checkAvailableProcessors(esClient PipelineLoader, requiredProcessors []Proc

 // LoadML loads the machine-learning configurations into Elasticsearch, if X-Pack is available
 func (reg *ModuleRegistry) LoadML(esClient PipelineLoader) error {
+	if !mlimporter.IsCompatible(esClient) {


Would it be also an option to return an error instead? Like this we make sure the user is aware of the change and can adjust the code.

What would be the expected action of the user? Change the setup command to exclude loading ML jobs?

What I have in mind is returning an error message when the user tried to load ML with the flag --machine-learning specified. Otherwise, just let them know that ML jobs are no longer loaded and do not return an error. WDYT?

If a user runs the setup command, are the machine learning jobs installed by default? If yes, agree we should break this behaviour. If it is only if a flag was set manually either through config for command line flags, we should likely tell the user about it by erroring out. What I worry is that users will not look at the logs and will be surprised at one point that nothing is loaded. At the same time this is not "system critical" I would expect.

Yes, it is loaded by default. I will add an error message for the case when users set machine learning explicitly.

ruflin · 2021-09-09T08:09:26Z

filebeat/fileset/modules.go

@@ -417,6 +422,11 @@ func (reg *ModuleRegistry) LoadML(esClient PipelineLoader) error {

 // SetupML sets up the machine-learning configurations into Elasticsearch using Kibana, if X-Pack is available
 func (reg *ModuleRegistry) SetupML(esClient PipelineLoader, kibanaClient *kibana.Client) error {
+	if !mlimporter.IsCompatible(esClient) {
+		logp.Info("Skipping loading machine learning jobs because of Elasticsearch version is too new.\nIt must be 7.x for setting up it using Beats. Please use the Machine Learning UI in Kibana.")


Looks like the exact same error message as above but I assume the two functions do something slightly different based on the description? Same question for error instead of log.

Yes, this function checks the ES version and the uses the Kibana client to load the ML jobs if it already has the required API.

libbeat/docs/command-reference.asciidoc

libbeat/ml-importer/importer.go

libbeat/docs/command-reference.asciidoc

mergify · 2021-09-10T13:18:45Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b backward-compatibility-setup-ml-jobs upstream/backward-compatibility-setup-ml-jobs
git merge upstream/7.x
git push upstream backward-compatibility-setup-ml-jobs

ruflin

Do we have any tests in place in 7.x that check for the ml setup to work?

I'm looking forward to the follow up PR to remove the code completely!

ruflin · 2021-09-21T07:01:33Z

filebeat/beater/filebeat.go

-func setupMLBasedOnVersion(reg *fileset.ModuleRegistry, esClient *eslegclient.Connection, kibanaClient *kibana.Client) error {
+func setupMLBasedOnVersion(reg *fileset.ModuleRegistry, fromFlag bool, esClient *eslegclient.Connection, kibanaClient *kibana.Client) error {
+	if !mlimporter.IsCompatible(esClient) && fromFlag {
+		return fmt.Errorf("Machine learning jobs are not loaded because Elasticsearch version is too new. Use the Machine learning UI in Kibana.")


Further down you mention that the version must be 7.x which is missing here.

mergify · 2021-09-28T15:15:05Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b backward-compatibility-setup-ml-jobs upstream/backward-compatibility-setup-ml-jobs
git merge upstream/7.x
git push upstream backward-compatibility-setup-ml-jobs

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 7, 2021

kvch added the Team:Elastic-Agent Label for the Agent team label Sep 7, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 7, 2021

kvch force-pushed the backward-compatibility-setup-ml-jobs branch from bd287c4 to a9c9da8 Compare September 8, 2021 11:08

kvch requested a review from ruflin September 8, 2021 15:02

ruflin reviewed Sep 9, 2021

View reviewed changes

dedemorton reviewed Sep 9, 2021

View reviewed changes

libbeat/docs/command-reference.asciidoc Outdated Show resolved Hide resolved

kvch force-pushed the backward-compatibility-setup-ml-jobs branch from a1a0d82 to 7dce2fe Compare September 14, 2021 14:46

ruflin approved these changes Sep 21, 2021

View reviewed changes

kvch force-pushed the backward-compatibility-setup-ml-jobs branch from ab1ec1f to b19155b Compare October 6, 2021 11:41

kvch added 7 commits October 6, 2021 15:14

Do not load ML jobs to next major version

f7256f2

add docs

a57dfdf

add changelog entry

bad4eb0

refer to ML docs

cd3b32b

add accidentally removed docs

0bdfd78

error when user specifies --machine-learning and es is too old

c29d0a9

add more details to error message

f5347df

kvch force-pushed the backward-compatibility-setup-ml-jobs branch from b19155b to f5347df Compare October 6, 2021 13:14

kvch merged commit b63fabd into elastic:7.x Oct 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not load ML jobs to 8.x from 7.x releases #27771

Do not load ML jobs to 8.x from 7.x releases #27771

kvch commented Sep 7, 2021 •

edited

Loading

elasticmachine commented Sep 7, 2021

elasticmachine commented Sep 7, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

kvch commented Sep 8, 2021

ruflin Sep 9, 2021

kvch Sep 10, 2021

ruflin Sep 10, 2021

kvch Sep 14, 2021

ruflin Sep 9, 2021

kvch Sep 10, 2021

mergify bot commented Sep 10, 2021

ruflin left a comment

ruflin Sep 21, 2021

mergify bot commented Sep 28, 2021

Do not load ML jobs to 8.x from 7.x releases #27771

Do not load ML jobs to 8.x from 7.x releases #27771

Conversation

kvch commented Sep 7, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Sep 7, 2021

elasticmachine commented Sep 7, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

kvch commented Sep 8, 2021

ruflin Sep 9, 2021

Choose a reason for hiding this comment

kvch Sep 10, 2021

Choose a reason for hiding this comment

ruflin Sep 10, 2021

Choose a reason for hiding this comment

kvch Sep 14, 2021

Choose a reason for hiding this comment

ruflin Sep 9, 2021

Choose a reason for hiding this comment

kvch Sep 10, 2021

Choose a reason for hiding this comment

mergify bot commented Sep 10, 2021

ruflin left a comment

Choose a reason for hiding this comment

ruflin Sep 21, 2021

Choose a reason for hiding this comment

mergify bot commented Sep 28, 2021

kvch commented Sep 7, 2021 •

edited

Loading

elasticmachine commented Sep 7, 2021 •

edited by jenkins-beats-ci bot

Loading