Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SO migration] remove v1 implementation #118000

Merged

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Nov 9, 2021

Summary

Fix #96396
Replace #98892

  • remove the SO migration v1 implementation
  • move the v2 implementation from src/core/server/saved_objects/migrationsv2 to src/core/server/saved_objects/migrations
  • minor cleanup and splitting

Checklist

@pgayvallet pgayvallet mentioned this pull request Nov 9, 2021
9 tasks
@pgayvallet pgayvallet added project:ResilientSavedObjectMigrations Reduce Kibana upgrade failures by making saved object migrations more resilient release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.1.0 labels Nov 9, 2021
@pgayvallet pgayvallet added v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Nov 10, 2021
Copy link
Contributor Author

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

self-review

@@ -1,222 +1,504 @@
# Saved Object Migrations
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old README was about the v1 migration, so I replaced it with the one that was previously in the migrationsv2 folder.

Comment on lines +34 to +37
export function disableUnknownTypeMappingFields(
activeMappings: IndexMapping,
sourceMappings: IndexMapping
): IndexMapping {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted from migration_context.ts, as this was the only function that is still used by v2.

Comment on lines +36 to +37
export const excludeUnusedTypesQuery: estypes.QueryDslQueryContainer = {
bool: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted from elastic_index.ts, same, these were the only bits that are still used by v2.

@@ -64,10 +63,8 @@ export class KibanaMigrator {
status: 'waiting_to_start',
});
private readonly activeMappings: IndexMapping;
// TODO migrationsV2: make private once we remove migrations v1
private readonly soMigrationsConfig: SavedObjectsMigrationConfigType;
public readonly kibanaVersion: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

kibanaVersion is actually still directly accessed from the SO repository

return getIndexForType({
type,
defaultIndex: this._index,
typeRegistry: this._registry,
kibanaVersion: this._migrator.kibanaVersion,

Comment on lines -23 to -27
export type MigrationLogLevel = 'error' | 'info' | 'warning';

export interface MigrationLog {
level: MigrationLogLevel;
message: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it make sense to split the state-related types from the other types, especially as src/core/server/saved_objects/migrations/types.ts was already present, so I created this state.ts file.

@@ -370,10 +370,10 @@ export class SavedObjectsService
};
}

public async start(
{ elasticsearch, pluginsInitialized = true }: SavedObjectsStartDeps,
migrationsRetryDelay?: number
Copy link
Contributor Author

Choose a reason for hiding this comment

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

migrationsRetryDelay was only used by v1 migration

Comment on lines -75 to -80
describe('Kibana index migration', () => {
before(() => esDeleteAllIndices('.migrate-*'));

it('Migrates an existing index that has never been migrated before', async () => {
const index = '.migration-a';
const originalDocs = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This must be a very old FTR test, as it's not really using the Kibana instance and was directly manipulating the IndexMigrator instead. As this class is no longer, I deleted the test suite, and as v2 is already properly covered in other FTR and IT tests suite, I did not add additional tests.

@pgayvallet pgayvallet marked this pull request as ready for review November 10, 2021 11:07
@pgayvallet pgayvallet requested a review from a team as a code owner November 10, 2021 11:07
@elasticmachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@pgayvallet pgayvallet merged commit f716387 into elastic:main Nov 10, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 10, 2021
* remove v1 implementation

* fix type

* remove unused mock

* expose kibanaVersion again

* fix migrator mock

* move KibanaMigrator out of the kibana subfolder

* fix imports

* moves migrationsv2 into migrations

* fix test mocking
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

@pgayvallet
Copy link
Contributor Author

(backporting to 8.0 to avoid conflicts with other migration-related changes/PRs, as this doesn't impact production code)

kibanamachine added a commit that referenced this pull request Nov 10, 2021
* remove v1 implementation

* fix type

* remove unused mock

* expose kibanaVersion again

* fix migrator mock

* move KibanaMigrator out of the kibana subfolder

* fix imports

* moves migrationsv2 into migrations

* fix test mocking

Co-authored-by: Pierre Gayvallet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed project:ResilientSavedObjectMigrations Reduce Kibana upgrade failures by making saved object migrations more resilient release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] Remove v1 migrations
4 participants