V2 migration algorithm: add tests for model versions by pgayvallet · Pull Request #158697 · elastic/kibana

pgayvallet · 2023-05-31T07:10:00Z

Summary

Add integration tests of scenarios using the v2 migration algorithm with SO types that are using the model version API

…tests

pgayvallet

Self-review

pgayvallet · 2023-05-31T12:39:22Z

...ts/core-saved-objects-migration-server-internal/src/document_migrator/validate_migrations.ts

+    if (Object.keys(modelVersionMap).length > 0) {
+      if (!type.switchToModelVersionAt) {
+        throw new Error(
+          `Type ${type.name}: Using modelVersions requires to specify switchToModelVersionAt`
+        );
+      }


Fixed a bug causing the validation to throw if an empty model version map was defined

This will throw for types that haven't switched to models on the currently running version yet. How do we handle that if a type doesn't need a migration when the switch happens? Will we not allow them to fall back to the most recent (non-model) migration?

The specific scenario I'm thinking about is:

Current kibana version: 8.11

switchToModelVersionsAt: 8.10

Most recent type migration implemented at 8.8. ie. The type didn’t need new migrations, only “legacy” ones.

We import a SO of that type from 8.7. (that's still allowed, right?)

On import, we run through migrations to get the existing object (on 8.7) to the current version (8.11). The object needs to run through the 8.8 migration but 8.8 < 8.10 (when the switch happened) and we don’t have a model version. How do we handle that?

I'm concerned that we're forcing folks to add a model when we force the switch over, even if the switch is postponed. Unless I'm missing something, we should have pre-emptively initialized a no-op as a model when we set the switch version. We didn't in #158267, an oversight?

This will throw for types that haven't switched to models on the currently running version yet. How do we handle that if a type doesn't need a migration when the switch happens?

No it won't. It's just that it was previously throwing when modelVersions: {} was defined when switchToModelVersionsAt wasn't, and it won't anymore.

You never were forced to define a model version when you switch. It's just that

you can't register "old" migrations for versions equal or higher to switchToModelVersionsAt, if set

you can't register modelVersions if switchToModelVersionsAt is not set

The object needs to run through the 8.8 migration but 8.8 < 8.10 (when the switch happened) and we don’t have a model version. How do we handle that?

As we were doing previously. The document will be upgraded to 8.8 given it's the latest register migration.

Keep in mind that the document migrator doesn't know or care about model versions. When model versions are registered, internally this part of the code only knows that some 10.x.0 migrations are present, and apply the exact same logic to them as it was previously doing.

FWIW your scenario is the exact same than if a customer was trying to import a 8.4 document into a 8.9 stack where the latest version of the type of the document is 8.6: it will upgrade the doc to 8.6 during import, and set the typeMigrationVersion accordingly to 8.6. So yes, this is already, and still will be, supported.

it will upgrade the doc to 8.6 during import, and set the typeMigrationVersion accordingly to 8.6. So yes, this is already, and still will be, supported.

Gold to my eyes! Thank you!

pgayvallet · 2023-05-31T12:47:20Z

src/core/server/integration_tests/saved_objects/migrations/group4/jest.integration.config.js

+  // see https://github.com/elastic/kibana/pull/130255/
+  preset: '@kbn/test/jest_integration',
+  rootDir: '../../../../../../..',
+  roots: ['<rootDir>/src/core/server/integration_tests/saved_objects/migrations/group4'],


Created a 4th group for the v2 integration tests

pgayvallet · 2023-05-31T12:48:09Z

...rver/integration_tests/saved_objects/migrations/group4/v2_with_mv_stack_version_bump.test.ts

+
+const NB_DOCS_PER_TYPE = 100;
+
+describe('V2 algorithm - using model versions - stack version bump scenario', () => {


Standard upgrade scenario where the stack version changes at the same time as the model versions of the registered types.

pgayvallet · 2023-05-31T12:48:40Z

...rver/integration_tests/saved_objects/migrations/group4/v2_with_mv_same_stack_version.test.ts

+
+const NB_DOCS_PER_TYPE = 25;
+
+describe('V2 algorithm - using model versions - upgrade without stack version increase', () => {


Standard upgrade scenario where the stack version doesn't change (only the model versions of the registered types change)

elasticmachine · 2023-05-31T12:53:59Z

Pinging @elastic/kibana-core (Team:Core)

TinaHeiligers

Changes look good and CI's happy.
I want to clarify if we still support running migrations for older documents on import before giving official approval.

pgayvallet · 2023-06-01T05:38:31Z

I want to clarify if we still support running migrations for older documents on import before giving official approval.

See my reply on the thread :)

TinaHeiligers

LGTM

…tests

kibana-ci · 2023-06-02T10:35:49Z

💚 Build Succeeded

Buildkite Build
Commit: 3049321

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	414	418	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	498	502	+4
total			+6

History

💚 Build #131264 succeeded 6b79593
💔 Build #131175 failed ba12409
💔 Build #131172 failed 66f8299

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

cc @pgayvallet

add base test

66f8299

pgayvallet added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes Feature:Migrations v8.9.0 labels May 31, 2023

pgayvallet added 4 commits May 31, 2023 09:23

add same stack version test

2661048

add test and update snapshot

75d8ee7

Merge remote-tracking branch 'upstream/main' into kbn-xxx-v2-algo-mv-…

ba12409

…tests

Merge remote-tracking branch 'upstream/main' into kbn-xxx-v2-algo-mv-…

6b79593

…tests

pgayvallet commented May 31, 2023

View reviewed changes

pgayvallet marked this pull request as ready for review May 31, 2023 12:53

pgayvallet requested a review from a team as a code owner May 31, 2023 12:53

TinaHeiligers reviewed May 31, 2023

View reviewed changes

afharo assigned pgayvallet Jun 1, 2023

TinaHeiligers self-requested a review June 1, 2023 17:51

TinaHeiligers approved these changes Jun 1, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/main' into kbn-xxx-v2-algo-mv-…

3049321

…tests

pgayvallet enabled auto-merge (squash) June 2, 2023 09:33

pgayvallet merged commit 1ba8be4 into elastic:main Jun 2, 2023

kibanamachine added the backport:skip This PR does not require backporting label Jun 2, 2023

rudolf added the Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects label Sep 23, 2023


		const NB_DOCS_PER_TYPE = 100;

		describe('V2 algorithm - using model versions - stack version bump scenario', () => {


		const NB_DOCS_PER_TYPE = 25;

		describe('V2 algorithm - using model versions - upgrade without stack version increase', () => {

Conversation

pgayvallet commented May 31, 2023

Summary

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet May 31, 2023

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers May 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

pgayvallet May 31, 2023

Choose a reason for hiding this comment

Uh oh!

pgayvallet May 31, 2023

Choose a reason for hiding this comment

Uh oh!

pgayvallet May 31, 2023

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented May 31, 2023

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Jun 2, 2023

💚 Build Succeeded

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TinaHeiligers May 31, 2023 •

edited

Loading

pgayvallet Jun 1, 2023 •

edited

Loading

pgayvallet Jun 1, 2023 •

edited

Loading

pgayvallet commented Jun 1, 2023 •

edited

Loading