Skip to content

APEX-72 Enforce Kibana >= 8.18.0; Remove switchToModelVersionAt#220985

Merged
gsoldevila merged 22 commits intoelastic:mainfrom
gsoldevila:cleanup-switch-to-model-version-at
Jun 20, 2025
Merged

APEX-72 Enforce Kibana >= 8.18.0; Remove switchToModelVersionAt#220985
gsoldevila merged 22 commits intoelastic:mainfrom
gsoldevila:cleanup-switch-to-model-version-at

Conversation

@gsoldevila
Copy link
Copy Markdown
Member

@gsoldevila gsoldevila commented May 20, 2025

Summary

The previous attempt caused a regression: index meta information started storing modelVersions that were older than the previously stored ones, which were defaulting to 10.0.0 for SO types that did NOT define modelVersions.

This was due to the removal of the applyTypeDefaults, which was ensuring all SOs had the switchToModelVersionAt property set.

This flag was then used by src/core/packages/saved-objects/base-server-internal/src/model_version/version_map.ts to determine whether to use modelVersions or the legacy migrations property in order to determine the latest model version for a given type.

When removing the switchToModelVersionAt flag (and its default backfill), the logic started defaulting to the latest migrations version for those SO types that were not defining any modelVersion, resulting in older versions that those stored in the SO indices. This caused incident https://elasticco.atlassian.net/browse/INC-3818.

This regression has been shipped in 9.0.0 (the PR was backported), so in top of the cleanup, we now need to address #220521 to ensure a smooth transition OnPrem => Serverless.

@gsoldevila gsoldevila 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 backport:version Backport to applied version labels v9.0.2 labels May 20, 2025
@gsoldevila gsoldevila force-pushed the cleanup-switch-to-model-version-at branch 3 times, most recently from 056a539 to ee13543 Compare May 30, 2025 09:22
@gsoldevila gsoldevila marked this pull request as ready for review May 30, 2025 09:22
@gsoldevila gsoldevila requested review from a team as code owners May 30, 2025 09:22
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@gsoldevila gsoldevila changed the title Remove legacy property switchToModelVersionAt Remove switchToModelVersionAt; Enforce Kibana >= 8.18.0 May 30, 2025
@botelastic botelastic bot added the Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. label May 30, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@TinaHeiligers TinaHeiligers self-requested a review June 1, 2025 20:12
@gsoldevila gsoldevila force-pushed the cleanup-switch-to-model-version-at branch from e620ba7 to f3b9db3 Compare June 2, 2025 07:52
@TinaHeiligers TinaHeiligers removed their request for review June 3, 2025 00:17
@gsoldevila gsoldevila changed the title Remove switchToModelVersionAt; Enforce Kibana >= 8.18.0 Enforce Kibana >= 8.18.0; Remove switchToModelVersionAt Jun 3, 2025
@rudolf rudolf added v9.0.3 and removed v9.0.2 labels Jun 3, 2025
Copy link
Copy Markdown
Contributor

@rudolf rudolf left a comment

Choose a reason for hiding this comment

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

haven't made it through the whole PR, but thought I'd leave this comment so long

buildType({
name: 'dolly',
modelVersions: {
0: dummyModelVersion(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we maybe do a runtime check to prevent modelversion 0? Otherwise you can define your type with just mappings and then at a later point define a transform or backfill modelVersion with 0 and I don't think that transform will be applied because the virtualVersionMap with useModelVersionsOnly=false stays 10.0.0.

I think a similar example might be bar with only migrations, if you add a modelVersion 0 then the virtual version still stays 10.0.0 so I think we'd miss mappings_addition.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Essentially I don't think this test will pass:

      expect(
        getLatestMappingsModelVersion(
          buildType({
            migrations: {
              '7.17.2': dummyMigration,
              '8.6.0': dummyMigration,
            }
          })
        )
      ).not.toEqual(
        getLatestMappingsModelVersion(
          buildType({
            migrations: {
              '7.17.2': dummyMigration,
              '8.6.0': dummyMigration,
            },
            modelVersions: {
              0: dummyModelVersionWithMappingsChanges(),
            },
          })
        )
      );

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I used it as a hack for this test, but fair point, I can try to limit the possible values for the keys.

@SiddharthMantri SiddharthMantri self-requested a review June 4, 2025 16:20
@gsoldevila gsoldevila self-assigned this Jun 6, 2025
switchToModelVersionAt: '8.9.0',
migrations: {
'8.9.0': jest.fn(),
'8.11.0': jest.fn(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

8.10.0 instead of 8.11.0?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, weird that it was like that 🤷🏼 I'll update it.

@gsoldevila gsoldevila force-pushed the cleanup-switch-to-model-version-at branch from 8ffb263 to 103cf2f Compare June 9, 2025 11:14
@gsoldevila gsoldevila force-pushed the cleanup-switch-to-model-version-at branch from aaf6256 to 386a784 Compare June 20, 2025 10:48
@gsoldevila gsoldevila requested a review from a team as a code owner June 20, 2025 10:48
@gsoldevila gsoldevila enabled auto-merge (squash) June 20, 2025 10:50
@gsoldevila gsoldevila merged commit 8efc526 into elastic:main Jun 20, 2025
10 checks passed
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

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 183 182 -1
@kbn/core-saved-objects-migration-server-internal 94 96 +2
@kbn/core-saved-objects-server 137 136 -1
total -0

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 886 887 +1
Unknown metric groups

API count

id before after diff
@kbn/core-saved-objects-migration-server-internal 127 130 +3
@kbn/core-saved-objects-server 574 572 -2
total +1

References to deprecated APIs

id before after diff
@kbn/core 638 633 -5

History

cc @gsoldevila

akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request Jun 25, 2025
…astic#220985)

## Summary

* Address elastic#217145 - Put in place
a check to ensure we're upgrading from Kibana 8.18.0 or newer.
* Address elastic#220521 - New attempt
at removing the `switchToModelVersionAt` property, inspired on
elastic#219029.

The previous attempt caused a regression: index meta information started
storing _modelVersions_ that were older than the previously stored ones,
which were defaulting to 10.0.0 for SO types that did NOT define
`modelVersions`.

This was due to the removal of the applyTypeDefaults, which was ensuring
all SOs had the `switchToModelVersionAt` property set.

This flag was then used by
`src/core/packages/saved-objects/base-server-internal/src/model_version/version_map.ts`
to determine whether to use `modelVersions` or the legacy `migrations`
property in order to determine the latest model version for a given
type.

When removing the `switchToModelVersionAt` flag (and its default
backfill), the logic started defaulting to the latest `migrations`
version for those SO types that were not defining any `modelVersion`,
resulting in older versions that those stored in the SO indices. This
caused incident https://elasticco.atlassian.net/browse/INC-3818.

This regression has been shipped in 9.0.0 (the PR was
[backported](elastic#219329)), so in top
of the cleanup, we now need to address
elastic#220521 to ensure a smooth
transition _OnPrem => Serverless_.

---------

Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Migrations release_note:skip Skip the PR/issue when compiling release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

APEX-72 Improve v2 logic to account for "older" migrationVersions in the index mappings Force Kibana upgrades to 8.18 / 8.19 before 9.x

9 participants