[8.8] Refactor KibanaMigrator, improve readability, maintainability and UT (#155693) #158953

gsoldevila · 2023-06-02T16:58:55Z

Backport

This will backport the following commits from main to 8.8:

Refactor KibanaMigrator, improve readability, maintainability and UT (#155693)

Questions ?

Please refer to the Backport tool documentation

…lastic#155693) Addresses the following feedback: elastic#154151 (comment) Similar to what has been done for ZDT, the goal of this PR is to extract the logic of the `runV2Migration()` from the `KibanaMigrator` into a separate file. The PR also fixes some incomplete / incorrect UTs and adds a few missing ones. (cherry picked from commit 06c337f) # Conflicts: # packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.test.ts # packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.ts # packages/core/saved-objects/core-saved-objects-migration-server-internal/src/run_resilient_migrator.ts # src/core/server/integration_tests/saved_objects/migrations/kibana_migrator_test_kit.ts

…-ref HEAD~1..HEAD --fix'

…fix'

TinaHeiligers

tiny nits but these don't need to go into this PR. I can go back after the fact and fix them.

TinaHeiligers · 2023-06-02T17:47:03Z

...ages/core/saved-objects/core-saved-objects-migration-server-internal/src/run_v2_migration.ts

+      options.logger.debug(`migrationVersion: ${migrationVersion} saved object type: ${type}`);
+    });
+
+  // build a indexTypesMap from the info present in tye typeRegistry, e.g.:


typo:

Suggested change

// build a indexTypesMap from the info present in tye typeRegistry, e.g.:

// build a indexTypesMap from the info present in typeRegistry, e.g.:

TinaHeiligers · 2023-06-02T17:50:16Z

...ages/core/saved-objects/core-saved-objects-migration-server-internal/src/run_v2_migration.ts

+
+  // build a list of all migrators that must be started
+  const migratorIndices = new Set(Object.keys(indexMap));
+  // indices involved in a relocation might no longer be present in current mappings


super nit: This sentence is confusing to me.

Suggested change

// indices involved in a relocation might no longer be present in current mappings

// the types in indices involved in relocation might not have mappings in the current mappings anymore

TinaHeiligers · 2023-06-02T17:50:57Z

...ages/core/saved-objects/core-saved-objects-migration-server-internal/src/run_v2_migration.ts

+
+  // compare indexTypesMap with the one present (or not) in the .kibana index meta
+  // and check if some SO types have been moved to different indices
+  const indicesWithMovingTypes = await getIndicesInvolvedInRelocation({


nit: consistent naming

Suggested change

const indicesWithMovingTypes = await getIndicesInvolvedInRelocation({

const indicesWithRelocatingTypes = await getIndicesInvolvedInRelocation({

TinaHeiligers · 2023-06-02T18:42:08Z

@gsoldevila From what I can see, the failures are from a type issue:
runOnNonMigratorNodes is declared in run_v2_migrations.test.ts and kibana_migrator.test.ts SO service config and the SO service config schema doesn't include it:

kibana/packages/core/saved-objects/core-saved-objects-base-server-internal/src/saved_objects_config.ts

Lines 40 to 42 in b7064bd

    
           zdt: schema.object({ 
        
             metaPickupSyncDelaySec: schema.number({ min: 1, defaultValue: 120 }), 
        
           }),

gsoldevila · 2023-06-02T20:28:56Z

@gsoldevila From what I can see, the failures are from a type issue: runOnNonMigratorNodes is declared in run_v2_migrations.test.ts and kibana_migrator.test.ts SO service config and the SO service config schema doesn't include it:

kibana/packages/core/saved-objects/core-saved-objects-base-server-internal/src/saved_objects_config.ts

Lines 40 to 42 in b7064bd

zdt: schema.object({

metaPickupSyncDelaySec: schema.number({ min: 1, defaultValue: 120 }),

}),

Thanks for the heads up! It seems like I backported that property and I shouldn't have. Fixing

…-ref HEAD~1..HEAD --fix'

…fix'

kibana-ci · 2023-06-03T14:20:01Z

💚 Build Succeeded

Buildkite Build
Commit: c15645b

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-saved-objects-base-server-internal`	52	54	+2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/core-saved-objects-base-server-internal`	9	8	-1

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-saved-objects-base-server-internal`	72	75	+3

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	17	19	+2
`securitySolution`	400	404	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	18	20	+2
`securitySolution`	480	484	+4
total			+6

History

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

gsoldevila · 2023-06-04T15:40:18Z

Feedback addressed in #158940

gsoldevila added the backport label Jun 2, 2023

gsoldevila enabled auto-merge (squash) June 2, 2023 16:58

gsoldevila mentioned this pull request Jun 2, 2023

Refactor KibanaMigrator, improve readability, maintainability and UT #155693

Merged

kibanamachine added 2 commits June 2, 2023 17:05

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

0f98404

…-ref HEAD~1..HEAD --fix'

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

3b3fe99

…fix'

TinaHeiligers requested review from TinaHeiligers, rudolf, afharo, jloleysens and a team and removed request for rudolf, afharo, jloleysens and TinaHeiligers June 2, 2023 17:17

TinaHeiligers approved these changes Jun 2, 2023

View reviewed changes

TinaHeiligers self-requested a review June 2, 2023 20:13

gsoldevila and others added 5 commits June 3, 2023 09:57

Fix backport issues

ea49cfd

Fix more types issues

c6cb807

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

f3bc396

…-ref HEAD~1..HEAD --fix'

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

1fdc0cc

…fix'

Fix remaining types issues

c15645b

gsoldevila merged commit a82751c into elastic:8.8 Jun 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[8.8] Refactor KibanaMigrator, improve readability, maintainability and UT (#155693) #158953

[8.8] Refactor KibanaMigrator, improve readability, maintainability and UT (#155693) #158953

gsoldevila commented Jun 2, 2023

TinaHeiligers left a comment

TinaHeiligers Jun 2, 2023

TinaHeiligers Jun 2, 2023

TinaHeiligers Jun 2, 2023

TinaHeiligers commented Jun 2, 2023

gsoldevila commented Jun 2, 2023

kibana-ci commented Jun 3, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

gsoldevila commented Jun 4, 2023

	// build a indexTypesMap from the info present in tye typeRegistry, e.g.:
	// build a indexTypesMap from the info present in typeRegistry, e.g.:

	// indices involved in a relocation might no longer be present in current mappings
	// the types in indices involved in relocation might not have mappings in the current mappings anymore

	const indicesWithMovingTypes = await getIndicesInvolvedInRelocation({
	const indicesWithRelocatingTypes = await getIndicesInvolvedInRelocation({

[8.8] Refactor KibanaMigrator, improve readability, maintainability and UT (#155693) #158953

[8.8] Refactor KibanaMigrator, improve readability, maintainability and UT (#155693) #158953

Conversation

gsoldevila commented Jun 2, 2023

Backport

Questions ?

TinaHeiligers left a comment

Choose a reason for hiding this comment

TinaHeiligers Jun 2, 2023

Choose a reason for hiding this comment

TinaHeiligers Jun 2, 2023

Choose a reason for hiding this comment

TinaHeiligers Jun 2, 2023

Choose a reason for hiding this comment

TinaHeiligers commented Jun 2, 2023

gsoldevila commented Jun 2, 2023

kibana-ci commented Jun 3, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Public APIs missing exports

API count

ESLint disabled line counts

Total ESLint disabled count

History

gsoldevila commented Jun 4, 2023