[ML][Fleet] Update Transform installation mechanism to support upgrade paths by qn895 · Pull Request #142920 · elastic/kibana

qn895 · 2022-10-06T21:03:59Z

Summary

Follow up of #140046. This PR:

Adds index aliases to the destination index for Transforms:
- It will automatically appends {{package-version}} name to the destination_index_name specified in transform.yml
- Create a {destination_index_name}.all that points to all the destination indices from all the previous versions and new version
- Create a {destination_index_name}.latest that points to just the destination index of the new version
Upgrading package to a new version no longer deletes the destination index
Downgrading package to an older version (e.g. from v3 to v1) will:
- Delete the transform from the newer version (v3), create transform for the older version (v1)
- If the older version was previously installed and the destination index of the older version v1 already exists: update the alias {destination_index_name}.latest to point to destination index v1.
- If the older version was never installed and destination index of the older version v1 does not exist: create the destination index with {destination_index_name}.all and {destination_index_name}.latest alias.
Support installing transforms concurrently and sequentially.
- If the order is specified in the transform.yml's _meta section, and all the numerical order are unique, transforms will be created and started sequentially. If not, they will be created and started concurrently.
Support versioning of transforms. If fleet_transform_version is specified in transform.yml's _meta section:
- If fleet_transform_version changed (either incremented or decremented): delete old transform, keep the old destination index, install new index templates, component templates, and transform
- If fleet_transform_version remains the same: keep old transform, keep the old destination index, do nothing new
Fixes an issue with the mappings and template not being applied to the destination index correctly when the destination index has an ingest pipeline. Previously, when the transform is associated with an ingest pipeline, we add the ingest pipeline to the settings when calling PUT index/{transform-destination-index}. This in turns makes the settings and mappings from the component templates not apply correctly to the destination. This PR changes so that it will add the pipeline to the component template.

Technical changes:

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…g aliases when first installed

droberts195 · 2022-11-09T09:27:48Z

x-pack/plugins/fleet/server/services/epm/elasticsearch/ingest_pipeline/remove.ts

+      // @TODO: Remove ignore 400
+      await esClient.ingest.deletePipeline({ id }, { ignore: [400] });


I think the way to remove the 400 is to set the index.default_pipeline setting on the old destination index to the empty string before deleting the pipeline. See elastic/elasticsearch#32286 (comment).

These changes go with those of elastic/kibana#142920. As we formalize the process by which the Fleet package installer will upgrade transforms more operations are required for managing the transforms and the related destination index: 1. Need to be able to add an alias on the transform destination index and adjust which indices it points to when upgrading the transform. 2. Need to be able to remove a default ingest pipeline from the settings of an old transform destination index during an upgrade that deletes the ingest pipeline.

…tes-being-overriden

These changes go with those of elastic/kibana#142920. As we formalize the process by which the Fleet package installer will upgrade transforms more operations are required for managing the transforms and the related destination index: 1. Need to be able to add an alias on the transform destination index and adjust which indices it points to when upgrading the transform. 2. Need to be able to remove a default ingest pipeline from the settings of an old transform destination index during an upgrade that deletes the ingest pipeline.

…tes-being-overriden

TinaHeiligers

Core related changes LGTM

kpollich

I stepped through all the code/tests (much appreciated on how many tests we've got for this process!) and everything looks good to me overall. I don't have a great way to test this on my end, though, so I'll defer to @kevinlog who I know was testing the Endpoint transforms previously.

I just had a few questions about whether we should be bubbling up granular errors for specific transform failures during installation - but I wouldn't consider those blocking questions here.

kpollich · 2022-12-08T14:17:20Z

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

+            logger.debug(`Created aliases for destination index: ${index}`);
+          }
        } catch (err) {
+          logger.debug(


Should this be logged to error or is this a non-blocking issue?

I have this same question about making errors visible to the user. It's generally a lot easier to debug with error messages than without!

Updated here e1a0050 (#142920)

kpollich · 2022-12-08T14:18:15Z

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

+      logger.debug(`Deleted alias: '${alias}' matching indices ${indicesMatchingAlias}`);
+    }
+  } catch (err) {
+    logger.debug(`Error deleting alias: ${alias}`);


Same here re: log levels.

Updated here e1a0050 (#142920)

droberts195

LGTM

kevinlog · 2022-12-19T21:27:09Z

@qn895 thank you for preparing this PR! I checked it out and tried it. See some observations below.

`kibana_system` permissions need to update

As discussed offline, since destination indices have the version appended to the end, we need to adjust the permissions for the kibana_system user in ES.

Initially, I saw the the following when trying to install the new transform schema with the Endpoint package.

Error: Error installing endpoint 8.7.1: security_exception: [security_exception] Reason: action [indices:admin/create] is unauthorized for user [kibana_system] with effective roles [kibana_system] on indices [.metrics-endpoint.metadata_united_default-8.7.1], this action is granted by the index privileges [create_index,manage,all]

I made the below change locally updating kibana_system permissions, and installation of transforms following the new schema works!

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java
index d8889003f36..df77b7660ad 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java
@@ -820,9 +820,9 @@ public class ReservedRolesStore implements BiConsumer<Set<String>, ActionListene
                     .build(),
                 RoleDescriptor.IndicesPrivileges.builder()
                     .indices(
-                        "metrics-endpoint.metadata_current_default",
-                        ".metrics-endpoint.metadata_current_default",
-                        ".metrics-endpoint.metadata_united_default"
+                        "metrics-endpoint.metadata_current_default*",
+                        ".metrics-endpoint.metadata_current_default*",
+                        ".metrics-endpoint.metadata_united_default*"
                     )
                     .privileges("create_index", "delete_index", "read", "index", IndicesAliasesAction.NAME, UpdateSettingsAction.NAME)
                     .build(),

Testing scenarios

I tested the following scenarios with the endpoint package.

Fresh package installs

Install legacy Endpoint package with legacy transforms in fresh install ✅
Install new schema Endpoint package with new transforms schema in fresh install ✅

Package upgrade scenarios

Upgrade legacy Endpoint package with legacy transforms to a newer legacy Endpoint package with legacy transforms (legacy -> legacy) ✅
Upgrade new schema Endpoint package with new transforms schema to a newer Endpoint package with new transforms schema (new schema -> new schema) ✅
- Note: I updated fleet_transform_version as specified to see the transform upgrade occur
Upgrade legacy Endpoint package with legacy transforms to a newer Endpoint package with new transforms schema (legacy -> new schema) ❌

In the (legacy -> new schema) upgrade test above, I am seeing that the new transforms get installed, but the legacy transforms are not being deleted. Further, the legacy destination indices are not being deleted. See some screenshots below to visualize what I'm seeing.

You see 2 sets of transforms. The ones prefixed logs-* are new schema. The ones without the logs-* prefix are legacy. I confirmed offline with @qn895 that the legacy transforms should be deleted.

You see 2 sets of destination indices below. The ones with the version at the end -8.9.1 are the new schema. The ones without the version at the end are legacy.

❓ It's understood that destination indices will now be kept around after upgrades between two packages both following the new transform schema. Should we delete the destination indices of the legacy transforms in the initial "cross over" upgrade to using the new schema for the first time? My initial opinion is that we should delete the legacy destination indices when we do the initial upgrade to the new schema to stay consistent with how legacy transforms worked and to have a cleaner "cross over". I'm interested to hear other opinions. cc @qn895 @droberts195 @joshdover @kpollich

Question about `{destination_index_name}.latest` alias and keeping old destination indices around

My understanding for the reasoning for keeping old destination indices around is to help with upgrade performance and re-indexing data. (Let me know if I have that right!)

For Endpoint purposes, we really only care about the latest data about which Endpoints are deployed. When testing the new upgrade paths, our Kibana API to display deployed Endpoints looks at our destination index and returns that list. With the new upgrade scenarios, our older destination indices will not be deleted. When our new transforms install, the new indices will be indexed with the latest documents representing deployed Endpoints. This will initially cause us to see duplicates in the UI because we'll get the same Endpoint ID from each destination index. To fix this, we'll update our API calls to point to the {destination_index_name}.latest alias to make sure we only get data from the latest index and avoid duplicates with stale data from older destination indices that are no longer updated.

Let me know if the above scenario and my usage of the {destination_index_name}.latest alias makes sense. My goal is to ensure that my interpretation of the intended usage of the aliases is in alignment with everyone. cc @qn895 @droberts195 @joshdover @kpollich

Changes to support elastic/kibana#142920. Transform destination indices now have a version number appended to their names, so the patterns used to set up the index privileges need to be adjusted accordingly.

…old schema to new

…tes-being-overriden

qn895 · 2022-12-20T20:54:06Z

Thanks Kevin for the thorough testing. I've updated the behavior for Upgrade legacy Endpoint package with legacy transforms to a newer Endpoint package with new transforms schema (legacy -> new schema) ❌ to reflect the discussed plan. Current behavior should now be:

Old transforms from version using legacy schema should now be deleted
Old destination indices from version using legacy schema should also now be deleted (I agree it's better for consistency)

kevinlog · 2022-12-20T22:36:18Z

@qn895 thank you for the update!

I checked it out and tried it again and it works!

I addition to the other testing scenarios, the below one now works.

Upgrade legacy Endpoint package with legacy transforms to a newer Endpoint package with new transforms schema (legacy -> new schema) ✅

LGTM!

qn895 · 2022-12-21T15:47:09Z

@elasticmachine merge upstream

Changes to support elastic/kibana#142920. Transform destination indices now have a version number appended to their names, so the patterns used to set up the index privileges need to be adjusted accordingly.

kibana-ci · 2022-12-21T16:48:50Z

💚 Build Succeeded

Buildkite Build
Commit: a99e62c

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`	119.7KB	119.8KB	+105.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/saved-objects-service.html#_mappings

id	before	after	diff
`epm-packages`	27	28	+1

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	61	67	+6
`osquery`	109	115	+6
`securitySolution`	439	445	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	70	76	+6
`osquery`	110	117	+7
`securitySolution`	515	521	+6
total			+21

History

💚 Build #96150 succeeded 76402ac
💚 Build #94860 succeeded e1a0050
💚 Build #93401 succeeded 306a2e1
💔 Build #93185 failed d6a22d2
💔 Build #93059 failed 8fb4a73
💔 Build #92894 failed 6984e15

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

cc @qn895

susan-shu-c · 2022-12-21T19:38:39Z

Thanks again for this 🙏 ☀️ 🎉

szeitlin · 2022-12-21T22:34:27Z

Thanks again for this 🙏 ☀️ 🎉

+1

[ML] Fix template being overridden

4a0ab13

qn895 changed the title ~~[ML] Fix template being overridden~~ [ML] Fix Fleet transforms installation's templates being overridden by index settings Oct 6, 2022

qn895 self-assigned this Oct 6, 2022

qn895 added the :ml label Oct 6, 2022

qn895 changed the title ~~[ML] Fix Fleet transforms installation's templates being overridden by index settings~~ [ML][Fleet] Update Transform installation mechanism to support upgrade paths Nov 2, 2022

qn895 force-pushed the ml-fleet-fix-templates-being-overriden branch from 1fe8c56 to a637796 Compare November 2, 2022 14:53

qn895 added 8 commits November 8, 2022 18:28

Clean up

93c87ac

Clean up

61f033e

Modify alias logic

f450566

Add logic for adding aliases, order of installing transforms, updatin…

adcbd4c

…g aliases when first installed

Wrap up for testing

8ca3a2e

Merge upstream/main into branch

af4db32

Remove @todos since they are addressed

36646d1

[TO REVERT] Ignore 400 for ingest pipeline deletion

8e2a6ff

qn895 force-pushed the ml-fleet-fix-templates-being-overriden branch from bf16496 to 8e2a6ff Compare November 9, 2022 02:37

qn895 requested review from droberts195 and kpollich November 9, 2022 02:40

droberts195 reviewed Nov 9, 2022

View reviewed changes

droberts195 mentioned this pull request Nov 10, 2022

Extra kibana_system privileges for Fleet transform upgrades elastic/elasticsearch#91499

Merged

susan-shu-c mentioned this pull request Nov 14, 2022

Create Host Risk Score package elastic/integrations#4019

Closed

4 tasks

qn895 added 3 commits November 14, 2022 17:56

Merge remote-tracking branch 'upstream/main' into ml-fleet-fix-templa…

49d1f59

…tes-being-overriden

Remove console

cf1e93b

[TODO] Update tests

ab3444f

qn895 added release_note:enhancement v8.7.0 labels Nov 16, 2022

qn895 added 3 commits December 5, 2022 09:38

Merge remote-tracking branch 'upstream/main' into ml-fleet-fix-templa…

0abab6d

…tes-being-overriden

Fix types, tests

ecbb8c1

Remove index as a fleet setting, add more tests

6984e15

Merge remote-tracking branch 'upstream/main' into ml-fleet-fix-templa…

306a2e1

…tes-being-overriden

qn895 requested a review from a team as a code owner December 7, 2022 19:17

TinaHeiligers approved these changes Dec 8, 2022

View reviewed changes

kpollich approved these changes Dec 8, 2022

View reviewed changes

droberts195 approved these changes Dec 12, 2022

View reviewed changes

qn895 added 2 commits December 14, 2022 15:04

Merge upstream into branch

f09f995

Change to debug.error

e1a0050

droberts195 mentioned this pull request Dec 20, 2022

More kibana_system changes to support Fleet transform install elastic/elasticsearch#92462

Merged

qn895 added 2 commits December 20, 2022 14:19

Update to delete old transform, and old dest index if upgrading from …

e56481c

…old schema to new

Merge remote-tracking branch 'upstream/main' into ml-fleet-fix-templa…

76402ac

…tes-being-overriden

kevinlog approved these changes Dec 20, 2022

View reviewed changes

Merge branch 'main' into ml-fleet-fix-templates-being-overriden

a99e62c

qn895 enabled auto-merge (squash) December 21, 2022 15:47

qn895 added the Feature:Transforms Transforms label Dec 21, 2022

qn895 merged commit b61066d into elastic:main Dec 21, 2022

kibanamachine added the backport:skip This PR does not require backporting label Dec 21, 2022

susan-shu-c mentioned this pull request Jan 3, 2023

[Fleet] Transform installation - order of dependencies #142891

Closed

qn895 mentioned this pull request Mar 8, 2023

[Cloud Posture] Install Transforms using package assets #151860

Open

3 tasks

qn895 deleted the ml-fleet-fix-templates-being-overriden branch March 28, 2023 15:45

susan-shu-c mentioned this pull request Jan 9, 2024

[Transform] Transforms with unattended flag don't create destination index unless all conditions/fields exist in source index elastic/elasticsearch#104146

Open

		// @TODO: Remove ignore 400
		await esClient.ingest.deletePipeline({ id }, { ignore: [400] });

Conversation

qn895 commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Risk Matrix

For maintainers

Uh oh!

droberts195 Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

kpollich left a comment

Choose a reason for hiding this comment

Uh oh!

kpollich Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

szeitlin Dec 14, 2022

Choose a reason for hiding this comment

Uh oh!

qn895 Dec 14, 2022

Choose a reason for hiding this comment

Uh oh!

kpollich Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

qn895 Dec 14, 2022

Choose a reason for hiding this comment

Uh oh!

droberts195 left a comment

Choose a reason for hiding this comment

Uh oh!

kevinlog commented Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kibana_system permissions need to update

Testing scenarios

Fresh package installs

Package upgrade scenarios

Question about {destination_index_name}.latest alias and keeping old destination indices around

Uh oh!

qn895 commented Dec 20, 2022

Uh oh!

kevinlog commented Dec 20, 2022

Uh oh!

qn895 commented Dec 21, 2022

Uh oh!

kibana-ci commented Dec 21, 2022

💚 Build Succeeded

Metrics [docs]

Page load bundle

Saved Objects .kibana field count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

susan-shu-c commented Dec 21, 2022

Uh oh!

szeitlin commented Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

qn895 commented Oct 6, 2022 •

edited

Loading

kevinlog commented Dec 19, 2022 •

edited

Loading

`kibana_system` permissions need to update

Question about `{destination_index_name}.latest` alias and keeping old destination indices around