[Fleet] Remove `upgradePackage` and consolidate it with `installPackage`, optimize calls to create index patterns #94490

jen-huang · 2021-03-11T20:54:41Z

Summary

This PR resolves Fleet plugin's portion of #91715 by removing direct calls to index pattern saved objects in server/services/epm/kibana/index_pattern/install.ts and replacing them with calls to data plugin's IndexPatternService.

Changes were made to data plugin's IndexPatternService to allow non-scripted fields to be returned from getAsSavedObjectBody when allowNoIndex is true: afc91ce. This allows the call to create and save the index pattern to work correctly.

March 17 update:

This PR no longer has any change to the way EPM installs index patterns, the current IndexPatternService does not work for the way EPM installs index patterns. We decided to pull out the changes I had here to get the service working for EPM in favor of having more discussions about potential solutions first, so unfortunately we still need to write directly to the index pattern saved objects themselves in the meantime 😞
However, this PR still sets us up to easily swap out SO for IndexPatternService when that's ready. Plus I think some of the simplifications introduced in this PR are beneficial for our error handling efforts, so I'd like to still get these changes in if the team agrees.

This PR introduces the following changes to EPM package installation:

Removes upgradePackage method, which was only being used by bulkInstallPackages, since installPackage can do the same thing (upgrade a package)
- Adjusts the code in bulkInstallPackages to use installPackage instead
- Preserve upgradePackage's use of handleInstallPackageFailure by adding it to installPackageFromRegistry
Adds new type InstallResult and use in various places where we previously had just AssetReference[], this new type will allow us to be more verbose about install results in the future, for now it includes an additional status: 'installed' | 'already_installed' property
Runs installIndexPatterns as a skippable post-install operation in installPackage, rather than in _installPackage()
- This allows bulkInstallPackages to skip this operation and run installIndexPatterns after all package installations are done to avoid race conditions that create duplicate index patterns (for example, on plugin setup call where we install multiple default packages, multiple logs-* and metrics-* patterns were created)
- ~~The race condition was not an issue before when we were overwriting directly to SO based on ID~~ We are keeping writing to SOs, but this still reduces the the amount of times we write to them
Removes our custom index pattern field typings in favor of the ones from data plugin and adjust our field generation code to return the correct field format
- analyzed and doc_value properties were removed for index pattern fields after offline discussions, they are not used by Kibana, they are preserved for index template fields
- type will default to string for unknown types since IndexPatternService does not support undefined type
Adds logging around modified areas

Even though there are quite a lot of changes here, everything around package installation and index patterns should behave the same way as before. To test this PR, ensure that the index patterns are created properly, with the right fields, whenever packages are installed, ungraded, or removed.

…rn creation & deletion, instead of interacting directly with index pattern SOs

…r of consolidating with installPackage(), use installPackage() for bulk install instead

…bulk package installation, install index pattern only if one or more packages are actually new installs, add debug logging

…Index is true

…when allowNoIndices is true

…-patterns

…tterns

…index-patterns" This reverts commit 8e712e9, reversing changes made to af0fb0e.

…Index is true (cherry picked from commit 69b93da)

…-patterns

This reverts commit b9770fd.

elasticmachine · 2021-03-16T21:33:27Z

Pinging @elastic/fleet (Team:Fleet)

mattkime · 2021-03-17T02:17:44Z

@jen-huang I'm skeptical but we can discuss it tomorrow.

skh · 2021-03-17T11:41:51Z

@skh FYI on the changes to package installation here. I know you are working on a related area re: handling upgrade failures, I tried to focus on simplification in this PR, so I hope these changes are actually beneficial for that work. Let me know if there is anything in this PR that conflicts with your work or is a concern.

Changes look good to me, good job on the simplification! I'll have to redo some bits from what I'm working on, but if this PR gets in reasonably quickly that's not a problem at all.

jonathan-buttner · 2021-03-17T19:46:04Z

@jen-huang I tested installing the endpoint package using the _bulk API and upgrading it and both worked:

Installing the package for the first time

Upgrading to 0.19.0

I noticed that the transform was being reinitialized on each _bulk API call ever after the latest package is installed though.

x-pack/plugins/fleet/server/services/epm/packages/install.ts

… allowNoIndex is true" This reverts commit afc91ce.

…-patterns

jen-huang · 2021-03-18T00:16:05Z

@elastic/fleet @jonathan-buttner Per discussion with @mattkime and others, we decided to table the changes to IndexPatternService for now, so this means EPM still needs to rely on writing directly to SOs. I've updated the code accordingly and updated PR description with more details. Let me know if any Qs or concerns about the rest of the changes. Thank you!

…-patterns

jen-huang · 2021-03-22T14:32:48Z

@elasticmachine merge upstream

kibanamachine · 2021-03-22T16:40:27Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`fleet`	344.7KB	344.4KB	-324.0B

History

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

cc @jen-huang

skh

I tested this locally and did not manage to break it, so I'd say let's get this in rather sooner than later 🚀

I very much like the simplifications in the package installation parts.

jen-huang · 2021-03-23T18:34:14Z

Thanks for testing this @skh, appreciate it! I will merge and backport this now.

…ge`, optimize calls to create index patterns (elastic#94490) * Add data plugin to server app context * First attempt at switching to indexPatternService for EPM index pattern creation & deletion, instead of interacting directly with index pattern SOs * Simplify bulk install package, remove upgradePackage() method in favor of consolidating with installPackage(), use installPackage() for bulk install instead * Update tests * Change cache logging of objects to trace level * Install index patterns as a post-package installation operation, for bulk package installation, install index pattern only if one or more packages are actually new installs, add debug logging * Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true * Allow `getFieldsForWildcard` to return fields saved on index pattern when allowNoIndices is true * Bring back passing custom ID for index pattern SO * Fix tests * Revert "Merge branch 'index-pattern/allow-no-index' into epm/missing-index-patterns" This reverts commit 8e712e9, reversing changes made to af0fb0e. * Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true (cherry picked from commit 69b93da) * Update API docs * Run post-install ops for install by upload too * Remove allowedInstallTypes param * Adjust force install conditions * Revert "Update API docs" This reverts commit b9770fd. * Revert "Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true" This reverts commit afc91ce. * Go back to using SO client for index patterns :( * Stringify attributes again for SO client saving * Fix condition for reinstall same pkg version * Remove unused type * Adjust comment * Update snapshot Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-03-23T18:36:57Z

💚 Backport successful

✅ 7.x / #95219

This backport PR will be merged automatically after passing CI.

…ge`, optimize calls to create index patterns (#94490) (#95219) * Add data plugin to server app context * First attempt at switching to indexPatternService for EPM index pattern creation & deletion, instead of interacting directly with index pattern SOs * Simplify bulk install package, remove upgradePackage() method in favor of consolidating with installPackage(), use installPackage() for bulk install instead * Update tests * Change cache logging of objects to trace level * Install index patterns as a post-package installation operation, for bulk package installation, install index pattern only if one or more packages are actually new installs, add debug logging * Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true * Allow `getFieldsForWildcard` to return fields saved on index pattern when allowNoIndices is true * Bring back passing custom ID for index pattern SO * Fix tests * Revert "Merge branch 'index-pattern/allow-no-index' into epm/missing-index-patterns" This reverts commit 8e712e9, reversing changes made to af0fb0e. * Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true (cherry picked from commit 69b93da) * Update API docs * Run post-install ops for install by upload too * Remove allowedInstallTypes param * Adjust force install conditions * Revert "Update API docs" This reverts commit b9770fd. * Revert "Allow getAsSavedObjectBody to return non-scripted fields when allowNoIndex is true" This reverts commit afc91ce. * Go back to using SO client for index patterns :( * Stringify attributes again for SO client saving * Fix condition for reinstall same pkg version * Remove unused type * Adjust comment * Update snapshot Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Jen Huang <[email protected]>

jen-huang added 6 commits March 10, 2021 10:58

Add data plugin to server app context

bc3ccc1

First attempt at switching to indexPatternService for EPM index patte…

9dec16e

…rn creation & deletion, instead of interacting directly with index pattern SOs

Simplify bulk install package, remove upgradePackage() method in favo…

5a09497

…r of consolidating with installPackage(), use installPackage() for bulk install instead

Update tests

480967d

Change cache logging of objects to trace level

4cb4f16

Install index patterns as a post-package installation operation, for …

0890eaa

…bulk package installation, install index pattern only if one or more packages are actually new installs, add debug logging

jen-huang self-assigned this Mar 11, 2021

jen-huang added 13 commits March 11, 2021 14:32

Allow getAsSavedObjectBody to return non-scripted fields when allowNo…

69b93da

…Index is true

Allow getFieldsForWildcard to return fields saved on index pattern …

1331321

…when allowNoIndices is true

Merge remote-tracking branch 'upstream/master' into epm/missing-index…

f5e3e06

…-patterns

Bring back passing custom ID for index pattern SO

af0fb0e

Merge branch 'index-pattern/allow-no-index' into epm/missing-index-pa…

8e712e9

…tterns

Fix tests

95ddd04

Revert "Merge branch 'index-pattern/allow-no-index' into epm/missing-…

76dd02b

…index-patterns" This reverts commit 8e712e9, reversing changes made to af0fb0e.

Allow getAsSavedObjectBody to return non-scripted fields when allowNo…

afc91ce

…Index is true (cherry picked from commit 69b93da)

Update API docs

b9770fd

Run post-install ops for install by upload too

19c7989

Remove allowedInstallTypes param

b013774

Adjust force install conditions

e3f2d03

Merge remote-tracking branch 'upstream/master' into epm/missing-index…

c2bf522

…-patterns

jen-huang added auto-backport Deprecated - use backport:version if exact versions are needed Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 v8.0.0 backport:skip This commit does not require backporting labels Mar 16, 2021

Revert "Update API docs"

c91092f

This reverts commit b9770fd.

jen-huang marked this pull request as ready for review March 16, 2021 21:33

jen-huang requested review from a team as code owners March 16, 2021 21:33

lizozom requested a review from mattkime March 17, 2021 15:58

jonathan-buttner reviewed Mar 17, 2021

View reviewed changes

x-pack/plugins/fleet/server/services/epm/packages/install.ts Outdated Show resolved Hide resolved

jen-huang added 5 commits March 17, 2021 15:14

Revert "Allow getAsSavedObjectBody to return non-scripted fields when…

1f61b8b

… allowNoIndex is true" This reverts commit afc91ce.

Go back to using SO client for index patterns :(

12fb018

Stringify attributes again for SO client saving

bf99ee1

Fix condition for reinstall same pkg version

9ee99bb

Merge remote-tracking branch 'upstream/master' into epm/missing-index…

e534f70

…-patterns

jen-huang removed request for a team and mattkime March 17, 2021 23:55

jen-huang changed the title ~~[Fleet] Use indexPatternService instead of writing directly to SOs~~ [Fleet] Remove upgradePackage and consolidate it with installPackage, optimize calls to create index patterns Mar 18, 2021

jen-huang added 2 commits March 17, 2021 17:18

Remove unused type

c7683ab

Adjust comment

cc069bf

jen-huang mentioned this pull request Mar 18, 2021

[Fleet] Use index pattern api to create/update index patterns #86121

Closed

jen-huang added 2 commits March 18, 2021 09:56

Update snapshot

1d845e3

Merge remote-tracking branch 'upstream/master' into epm/missing-index…

54ff3ac

…-patterns

Merge branch 'master' into epm/missing-index-patterns

f12ddfc

skh approved these changes Mar 23, 2021

View reviewed changes

jen-huang merged commit d886979 into elastic:master Mar 23, 2021

jen-huang deleted the epm/missing-index-patterns branch March 23, 2021 18:34

kibanamachine mentioned this pull request Mar 23, 2021

[7.x] [Fleet] Remove upgradePackage and consolidate it with installPackage, optimize calls to create index patterns (#94490) #95219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Remove `upgradePackage` and consolidate it with `installPackage`, optimize calls to create index patterns #94490

[Fleet] Remove `upgradePackage` and consolidate it with `installPackage`, optimize calls to create index patterns #94490

jen-huang commented Mar 11, 2021 •

edited

Loading

elasticmachine commented Mar 16, 2021

mattkime commented Mar 17, 2021

skh commented Mar 17, 2021

jonathan-buttner commented Mar 17, 2021 •

edited

Loading

jen-huang commented Mar 18, 2021

jen-huang commented Mar 22, 2021

kibanamachine commented Mar 22, 2021

skh left a comment

jen-huang commented Mar 23, 2021

kibanamachine commented Mar 23, 2021

[Fleet] Remove upgradePackage and consolidate it with installPackage, optimize calls to create index patterns #94490

[Fleet] Remove upgradePackage and consolidate it with installPackage, optimize calls to create index patterns #94490

Conversation

jen-huang commented Mar 11, 2021 • edited Loading

Summary

elasticmachine commented Mar 16, 2021

mattkime commented Mar 17, 2021

skh commented Mar 17, 2021

jonathan-buttner commented Mar 17, 2021 • edited Loading

jen-huang commented Mar 18, 2021

jen-huang commented Mar 22, 2021

kibanamachine commented Mar 22, 2021

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

skh left a comment

Choose a reason for hiding this comment

jen-huang commented Mar 23, 2021

kibanamachine commented Mar 23, 2021

💚 Backport successful

[Fleet] Remove `upgradePackage` and consolidate it with `installPackage`, optimize calls to create index patterns #94490

[Fleet] Remove `upgradePackage` and consolidate it with `installPackage`, optimize calls to create index patterns #94490

jen-huang commented Mar 11, 2021 •

edited

Loading

jonathan-buttner commented Mar 17, 2021 •

edited

Loading