Skip to content

Commit

Permalink
[ZDT] add aliases when creating the index (#179792)
Browse files Browse the repository at this point in the history
## Summary

Fix #179783

Create the aliases during initial index creation, instead of doing it in
a later stage. This avoids an additional roundtrip with ES during
project creation.

**Before:** 
```
INIT -> CREATE_TARGET_INDEX -> UPDATE_ALIASES -> INDEX_STATE_UPDATE_DONE -> DONE
```

**After:** 
```
INIT -> CREATE_TARGET_INDEX -> INDEX_STATE_UPDATE_DONE -> DONE
```
  • Loading branch information
pgayvallet authored Apr 2, 2024
1 parent 6ecbe53 commit 8b95195
Show file tree
Hide file tree
Showing 14 changed files with 129 additions and 81 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import { getAliasActionsMock } from './create_target_index.test.mocks';
import * as Either from 'fp-ts/lib/Either';
import {
createContextMock,
Expand All @@ -24,12 +23,12 @@ describe('Stage: createTargetIndex', () => {
...createPostInitState(),
controlState: 'CREATE_TARGET_INDEX',
indexMappings: { properties: { foo: { type: 'text' } }, _meta: {} },
creationAliases: [],
...parts,
});

beforeEach(() => {
context = createContextMock();
getAliasActionsMock.mockReset().mockReturnValue([]);
});

describe('In case of left return', () => {
Expand Down Expand Up @@ -68,50 +67,11 @@ describe('Stage: createTargetIndex', () => {
});

describe('In case of right return', () => {
it('calls getAliasActions with the correct parameters', () => {
const state = createState();
const res: StateActionResponse<'CREATE_TARGET_INDEX'> =
Either.right('create_index_succeeded');

createTargetIndex(state, res, context);

expect(getAliasActionsMock).toHaveBeenCalledTimes(1);
expect(getAliasActionsMock).toHaveBeenCalledWith({
currentIndex: state.currentIndex,
existingAliases: [],
indexPrefix: context.indexPrefix,
kibanaVersion: context.kibanaVersion,
});
});

it('CREATE_TARGET_INDEX -> UPDATE_ALIASES when successful and alias actions are not empty', () => {
const state = createState();
const res: StateActionResponse<'CREATE_TARGET_INDEX'> =
Either.right('create_index_succeeded');

const aliasActions = [{ add: { index: '.kibana_1', alias: '.kibana' } }];
getAliasActionsMock.mockReturnValue(aliasActions);

const newState = createTargetIndex(state, res, context);

expect(newState).toEqual({
...state,
controlState: 'UPDATE_ALIASES',
previousMappings: state.indexMappings,
currentIndexMeta: state.indexMappings._meta,
aliases: [],
aliasActions,
skipDocumentMigration: true,
});
});

it('CREATE_TARGET_INDEX -> INDEX_STATE_UPDATE_DONE when successful and alias actions are empty', () => {
const state = createState();
it('CREATE_TARGET_INDEX -> INDEX_STATE_UPDATE_DONE when successful', () => {
const state = createState({});
const res: StateActionResponse<'CREATE_TARGET_INDEX'> =
Either.right('create_index_succeeded');

getAliasActionsMock.mockReturnValue([]);

const newState = createTargetIndex(state, res, context);

expect(newState).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import { delayRetryState } from '../../../model/retry_state';
import { throwBadResponse } from '../../../model/helpers';
import { CLUSTER_SHARD_LIMIT_EXCEEDED_REASON } from '../../../common/constants';
import { isTypeof } from '../../actions';
import { getAliasActions } from '../../utils';
import type { ModelStage } from '../types';

export const createTargetIndex: ModelStage<
'CREATE_TARGET_INDEX',
'UPDATE_ALIASES' | 'INDEX_STATE_UPDATE_DONE' | 'FATAL'
'INDEX_STATE_UPDATE_DONE' | 'FATAL'
> = (state, res, context) => {
if (Either.isLeft(res)) {
const left = res.left;
Expand All @@ -36,22 +35,15 @@ export const createTargetIndex: ModelStage<
}
}

const aliasActions = getAliasActions({
currentIndex: state.currentIndex,
existingAliases: [],
indexPrefix: context.indexPrefix,
kibanaVersion: context.kibanaVersion,
});

const currentIndexMeta = cloneDeep(state.indexMappings._meta!);

return {
...state,
controlState: aliasActions.length ? 'UPDATE_ALIASES' : 'INDEX_STATE_UPDATE_DONE',
controlState: 'INDEX_STATE_UPDATE_DONE',
previousMappings: state.indexMappings,
currentIndexMeta,
aliases: [],
aliasActions,
aliasActions: [],
skipDocumentMigration: true,
previousAlgorithm: 'zdt',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const generateAdditiveMappingDiffMock = jest.fn();
export const getAliasActionsMock = jest.fn();
export const checkIndexCurrentAlgorithmMock = jest.fn();

export const getCreationAliasesMock = jest.fn();

jest.doMock('../../utils', () => {
const realModule = jest.requireActual('../../utils');
return {
Expand All @@ -23,6 +25,7 @@ jest.doMock('../../utils', () => {
generateAdditiveMappingDiff: generateAdditiveMappingDiffMock,
getAliasActions: getAliasActionsMock,
checkIndexCurrentAlgorithm: checkIndexCurrentAlgorithmMock,
getCreationAliases: getCreationAliasesMock,
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getAliasActionsMock,
checkIndexCurrentAlgorithmMock,
getAliasesMock,
getCreationAliasesMock,
} from './init.test.mocks';
import * as Either from 'fp-ts/lib/Either';
import { FetchIndexResponse } from '../../../actions';
Expand Down Expand Up @@ -57,6 +58,7 @@ describe('Stage: init', () => {
checkIndexCurrentAlgorithmMock.mockReset().mockReturnValue('zdt');
getAliasesMock.mockReset().mockReturnValue(Either.right({}));
buildIndexMappingsMock.mockReset().mockReturnValue({});
getCreationAliasesMock.mockReset().mockReturnValue([]);

context = createContextMock({ indexPrefix: '.kibana', types: ['foo', 'bar'] });
context.typeRegistry.registerType({
Expand Down Expand Up @@ -310,6 +312,20 @@ describe('Stage: init', () => {
});
});

it('calls getCreationAliases with the correct parameters', () => {
const state = createState();
const fetchIndexResponse = createResponse();
const res: StateActionResponse<'INIT'> = Either.right(fetchIndexResponse);

init(state, res, context);

expect(getCreationAliasesMock).toHaveBeenCalledTimes(1);
expect(getCreationAliasesMock).toHaveBeenCalledWith({
indexPrefix: context.indexPrefix,
kibanaVersion: context.kibanaVersion,
});
});

it('INIT -> CREATE_TARGET_INDEX', () => {
const state = createState();
const fetchIndexResponse = createResponse();
Expand All @@ -318,13 +334,17 @@ describe('Stage: init', () => {
const mockMappings = { properties: { someMappings: 'string' } };
buildIndexMappingsMock.mockReturnValue(mockMappings);

const creationAliases = ['.foo', '.bar'];
getCreationAliasesMock.mockReturnValue(creationAliases);

const newState = init(state, res, context);

expect(newState).toEqual(
expect.objectContaining({
controlState: 'CREATE_TARGET_INDEX',
currentIndex: '.kibana_1',
indexMappings: mockMappings,
creationAliases,
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
checkVersionCompatibility,
buildIndexMappings,
getAliasActions,
getCreationAliases,
generateAdditiveMappingDiff,
checkIndexCurrentAlgorithm,
removePropertiesFromV2,
Expand Down Expand Up @@ -73,6 +74,10 @@ export const init: ModelStage<
controlState: 'CREATE_TARGET_INDEX',
currentIndex: `${context.indexPrefix}_1`,
indexMappings: buildIndexMappings({ types }),
creationAliases: getCreationAliases({
indexPrefix: context.indexPrefix,
kibanaVersion: context.kibanaVersion,
}),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ import { nextActionMap, type ActionMap } from './next';
import {
createContextMock,
type MockedMigratorContext,
createPostInitState,
createPostDocInitState,
} from './test_helpers';
import type {
SetDocMigrationStartedState,
UpdateMappingModelVersionState,
UpdateDocumentModelVersionsState,
UpdateIndexMappingsState,
CreateTargetIndexState,
} from './state';

describe('actions', () => {
Expand Down Expand Up @@ -73,6 +75,33 @@ describe('actions', () => {
});
});

describe('CREATE_TARGET_INDEX', () => {
it('calls createIndex with the correct parameters', () => {
const state: CreateTargetIndexState = {
...createPostInitState(),
controlState: 'CREATE_TARGET_INDEX',
currentIndex: '.kibana_1',
indexMappings: {
properties: { foo: { type: 'keyword' } },
},
creationAliases: ['.kibana', '.kibana_foo'],
};

const action = actionMap.CREATE_TARGET_INDEX;

action(state);

expect(ActionMocks.createIndex).toHaveBeenCalledTimes(1);
expect(ActionMocks.createIndex).toHaveBeenCalledWith({
client: context.elasticsearchClient,
indexName: state.currentIndex,
aliases: state.creationAliases,
mappings: state.indexMappings,
esCapabilities: context.esCapabilities,
});
});
});

describe('UPDATE_MAPPING_MODEL_VERSIONS', () => {
it('calls setMetaMappingMigrationComplete with the correct parameters', () => {
const state: UpdateMappingModelVersionState = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const nextActionMap = (context: MigratorContext) => {
Actions.createIndex({
client,
indexName: state.currentIndex,
aliases: state.creationAliases,
mappings: state.indexMappings,
esCapabilities: context.esCapabilities,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export interface CreateTargetIndexState extends BaseState {
readonly controlState: 'CREATE_TARGET_INDEX';
readonly currentIndex: string;
readonly indexMappings: IndexMapping;
readonly creationAliases: string[];
}

export interface UpdateIndexMappingsState extends PostInitState {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { getCreationAliases } from './get_creation_aliases';

describe('getCreationAliases', () => {
it('returns the correct list of alias', () => {
const aliases = getCreationAliases({ indexPrefix: '.kibana', kibanaVersion: '8.13.0' });
expect(aliases).toEqual(['.kibana', '.kibana_8.13.0']);
});

it('returns the correct version alias', () => {
const aliases = getCreationAliases({ indexPrefix: '.kibana', kibanaVersion: '8.17.2' });
expect(aliases).toEqual(['.kibana', '.kibana_8.17.2']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

interface GetAliasActionOpts {
indexPrefix: string;
kibanaVersion: string;
}

/**
* Build the list of alias actions to perform, depending on the current state of the cluster.
*/
export const getCreationAliases = ({
indexPrefix,
kibanaVersion,
}: GetAliasActionOpts): string[] => {
const globalAlias = indexPrefix;
const versionAlias = `${indexPrefix}_${kibanaVersion}`;
return [globalAlias, versionAlias];
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ export {
checkIndexCurrentAlgorithm,
type CheckCurrentAlgorithmResult,
} from './check_index_algorithm';
export { getCreationAliases } from './get_creation_aliases';
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,14 @@ describe('ZDT upgrades - running on a fresh cluster', () => {

const records = await parseLogFile(logFilePath);

expect(records).toContainLogEntry('INIT -> CREATE_TARGET_INDEX');
expect(records).toContainLogEntry('CREATE_TARGET_INDEX -> UPDATE_ALIASES');
expect(records).toContainLogEntry('UPDATE_ALIASES -> INDEX_STATE_UPDATE_DONE');
expect(records).toContainLogEntry('INDEX_STATE_UPDATE_DONE -> DONE');
expect(records).toContainLogEntry('Migration completed');
expect(records).toContainLogEntries(
[
'INIT -> CREATE_TARGET_INDEX',
'CREATE_TARGET_INDEX -> INDEX_STATE_UPDATE_DONE',
'INDEX_STATE_UPDATE_DONE -> DONE',
'Migration completed',
],
{ ordered: true }
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,14 @@ describe('ZDT with v2 compat - running on a fresh cluster', () => {

const records = await parseLogFile(logFilePath);

expect(records).toContainLogEntry('INIT -> CREATE_TARGET_INDEX');
expect(records).toContainLogEntry('CREATE_TARGET_INDEX -> UPDATE_ALIASES');
expect(records).toContainLogEntry('UPDATE_ALIASES -> INDEX_STATE_UPDATE_DONE');
expect(records).toContainLogEntry('INDEX_STATE_UPDATE_DONE -> DONE');
expect(records).toContainLogEntry('Migration completed');
expect(records).toContainLogEntries(
[
'INIT -> CREATE_TARGET_INDEX',
'CREATE_TARGET_INDEX -> INDEX_STATE_UPDATE_DONE',
'INDEX_STATE_UPDATE_DONE -> DONE',
'Migration completed',
],
{ ordered: true }
);
});
});

0 comments on commit 8b95195

Please sign in to comment.