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.

Sign up for GitHub

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

Jump to bottom

[8.6][ML Inference] Verify pipeline usage before deletion #144053

Merged
merged 52 commits into from
Oct 28, 2022
Merged

[8.6][ML Inference] Verify pipeline usage before deletion #144053

merged 52 commits into from
Oct 28, 2022

Conversation

demjened
Copy link
Contributor

@demjened demjened commented Oct 26, 2022 •
edited
Loading

Summary

This PR adds a check to the DELETE /internal/enterprise_search/indices/{indexName}/ml_inference/pipeline_processors/{pipelineName} endpoint. Before deleting the supplied ML inference processor pipeline from the given index, we check whether the same pipeline is referenced in a different index (or more precisely, in the {other_index_name}@ml-inference managed pipeline).

Manual testing

ml-inference-ner2 is referenced in two indices' managed pipelines:
Screen Shot 2022-10-26 at 14 20 01
Screen Shot 2022-10-26 at 14 20 20

Deleting the ml-inference-ner3 pipeline succeeds

Screen Shot 2022-10-26 at 14 20 41

Deleting the ml-inference-ner2 pipeline fails

Screen Shot 2022-10-27 at 15 09 42

Checklist

demjened and others added 30 commits September 13, 2022 13:47
@demjened
Add ML inference PL creation flow
c9a0545
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
e293860 
…-fix'
@demjened
Add exists check, clean up code a bit
8fb6d63
@demjened
Merge branch 'create-ml-inference-pipeline' of https://github.com/dem…
05f5eb2 
…jened/kibana into create-ml-inference-pipeline
@demjened
Fix dest name
d69478b
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
2443d8c 
…-fix'
@demjened
Separate concerns
49aebdb
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
aee672f 
…-ref HEAD~1..HEAD --fix'
@demjened
Remove i18n due to linter error, fix src field ref
d838052
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
f4d53ba 
…-ref HEAD~1..HEAD --fix'
@demjened
Add/update unit tests
ef14bad
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
5d3b37d 
…-ref HEAD~1..HEAD --fix'
@demjened
Refactor error handling
8613f5b
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
b972688 
…-fix'
@demjened
Add sub-pipeline to parent ML PL
92b4ec5
@demjened
Add unit tests and docs
71bf27b
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
7c74509 
…-ref HEAD~1..HEAD --fix'
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
0ecad7e 
…-fix'
@demjened
Merge branch 'main' into create-ml-inference-pipeline
ce8f0a7
@demjened
Refactor error handling
1c15559
@demjened
Wrap logic into higher level function
bf29f2a
@demjened
Add route test
aaae199
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
9d2f232 
…-ref HEAD~1..HEAD --fix'
@demjened
Merge branch 'main' into create-ml-inference-pipeline
99a7027
@kibanamachine
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
63c52cb 
…-fix'
@demjened
Merge branch 'create-ml-inference-pipeline'
85a5020
@demjened
Merge branch 'main' of https://github.com/elastic/kibana
1f70e59
@demjened
Merge branch 'main' of https://github.com/elastic/kibana
58f6524
@demjened
Merge branch 'main' of https://github.com/elastic/kibana
27028f8
@demjened
Merge branch 'main' of https://github.com/elastic/kibana
8acd6a3
@demjened demjened added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:EnterpriseSearch v8.6.0 labels Oct 26, 2022
@demjened demjened marked this pull request as ready for review October 26, 2022 18:34
@demjened demjened requested a review from a team October 26, 2022 18:34
brianmcgue
brianmcgue reviewed Oct 26, 2022
View reviewed changes
Copy link
Contributor

@brianmcgue brianmcgue left a comment

Choose a reason for hiding this comment

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

A few comments/questions, but overall, LGTM

x-pack/plugins/enterprise_search/server/routes/enterprise_search/indices.ts Outdated Show resolved Hide resolved
x-pack/plugins/enterprise_search/server/routes/enterprise_search/indices.test.ts Outdated Show resolved Hide resolved
...erver/lib/indices/pipelines/ml_inference/pipeline_processors/delete_ml_inference_pipeline.ts Outdated Show resolved Hide resolved
@demjened demjened force-pushed the demjened/validate-pipeline-usage branch 2 times, most recently from 9a9247a to fbb10bf Compare October 28, 2022 16:20
@demjened demjened force-pushed the demjened/validate-pipeline-usage branch from fbb10bf to 5843600 Compare October 28, 2022 16:20
@demjened demjened force-pushed the demjened/validate-pipeline-usage branch from 5843600 to e86622a Compare October 28, 2022 17:53
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 1.7MB 1.7MB +42.0B

History

  • 💔 Build #83612 failed 58436000024fd987305034ffe7d0d7c21c0826fb
  • 💔 Build #83569 failed 9a9247a9254414a8a80a4d449a69d843e06ab12a
  • 💔 Build #83336 failed 94afd27682519225661e634c4dfad2e582dd23a1
  • 💔 Build #83329 failed f52568e79e945f354342573ad325975a97055590
  • 💚 Build #82931 succeeded 66d007aba669d5cceb02a724ad0631eb0776cc7b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@demjened demjened merged commit 7671176 into elastic:main Oct 28, 2022
@demjened demjened deleted the demjened/validate-pipeline-usage branch October 28, 2022 20:13
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 31, 2022
@jloleysens
Merge branch 'main' into files-move-to-src
866c302 
* main: (41 commits)
  [api-docs] Daily api_docs build (elastic#144212)
  Add readonly view to role management (elastic#143893)
  [api-docs] Daily api_docs build (elastic#144208)
  [APM] Adds button group to navigate to "All services" (elastic#142911)
  Update react-query to ^4.12.0 (main) (elastic#139986)
  [APM] Support specific fields when creating service groups (elastic#142201) (elastic#143881)
  [api-docs] Daily api_docs build (elastic#144203)
  [ts] add stub index.d.ts in @kbn/ui-shared-deps-npm
  [Synthetics] Fix failing Synthetics Integration test (elastic#144175)
  chore(NA): remove @types/pkg link creation when generating a new package (elastic#144200)
  [Osquery] Update schema to v5.5.1 (elastic#144090)
  [ci] remove github-checks-reporter (elastic#144193)
  [8.6][ML Inference] Verify pipeline usage before deletion (elastic#144053)
  [ts] ts refs cache was removed, remove capture task
  Added Rollups CCS Test (elastic#144074)
  [auto] migrate existing plugin/package configs
  [ts] stop building @types packages in bootstrap
  skip failing test suite (elastic#142762)
  skip failing test suite (elastic#144186)
  [Fleet] Show Add Fleet Server instead of add agent when adding agent from agent policy (elastic#144105)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianmcgue brianmcgue brianmcgue approved these changes

@TattdCodeMonkey TattdCodeMonkey Awaiting requested review from TattdCodeMonkey

@sphilipse sphilipse Awaiting requested review from sphilipse

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@demjened @kibana-ci @brianmcgue @sphilipse @kibanamachine