Skip to content

Support ML module Kibana type#152

Merged
jgowdyelastic merged 6 commits intoelastic:masterfrom
jgowdyelastic:add-support-for-ml-modules
Mar 24, 2021
Merged

Support ML module Kibana type#152
jgowdyelastic merged 6 commits intoelastic:masterfrom
jgowdyelastic:add-support-for-ml-modules

Conversation

@jgowdyelastic
Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic commented Mar 18, 2021

Adds support for ML module asset types.

ML Module's are now supported in the form of saved objects in kibana. This PR adds the support for installing the modules via Fleet's integration packages.

Once a Module has been installed via Fleet, it is available for use in ML. This involved calling ML's "setup" endpoint which will create the contained Jobs, with optional overrides, and then optionally starting the Jobs.

An ML Module installed via Fleet should be free to be deleted or upgraded with all other assets in the package.
Deleting an ML Module will not remove any previously created Jobs.
Upgrading the ML Module will not update or affect in any way any previously created Jobs.

Related issues

Related to:
#148
elastic/kibana#92855
elastic/kibana#94950

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Mar 18, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #152 updated

  • Start Time: 2021-03-23T15:38:33.739+0000

  • Duration: 5 min 1 sec

  • Commit: 58f8cb6

Trends 🧪

Image of Build Times

@ycombinator
Copy link
Copy Markdown
Contributor

Hi @jgowdyelastic, it's not obvious, but updating the changelog requires another make update as the changelog.yml file is part of the spec folder.

@ruflin ruflin requested a review from mtojek March 22, 2021 10:10
Copy link
Copy Markdown
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. But would be good to have also input from @mtojek and @ycombinator on this.

@jgowdyelastic One thing that would be useful is having just a quick note in the PR description on how this assets should be installed / removed / upgrade. I know you already have an open PR against Kibana that describes this in code. But someone coming back to the package spec change and wants to know how this assets work, it would be useful to have it here.

@@ -0,0 +1 @@
{} No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would be nice to have some example content, mostly to see how it will look and as an example.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've added the NGINX module saved object in 05395ed

pattern: '^.+\.json$'
- description: Folder containing ML module assets
type: folder
name: ml_module
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator Mar 22, 2021

Choose a reason for hiding this comment

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

Looking at the test package, I assume you want the folder under kibana/ to be named ml/?

Suggested change
name: ml_module
name: ml

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the folder should be under kibana but named ml_module. There might be other ML assets in the future.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes this is a mistake. The changes I had made to test elastic/kibana#94950 use ml_module
Updated in 92fab43

@ruflin
Copy link
Copy Markdown
Collaborator

ruflin commented Mar 23, 2021

@jgowdyelastic Is this ready for review?

@jgowdyelastic jgowdyelastic self-assigned this Mar 23, 2021
@jgowdyelastic jgowdyelastic added the enhancement New feature or request label Mar 23, 2021
@jgowdyelastic jgowdyelastic marked this pull request as ready for review March 23, 2021 11:53
@jgowdyelastic
Copy link
Copy Markdown
Member Author

@jgowdyelastic Is this ready for review?

@ruflin It is now! You were all so keen to look at it I hadn't had time to update the description or test it before taking it out of draft.
Thanks for taking a look.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@jgowdyelastic
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants