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.

Sign up for GitHub

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

Add migration support to the event log #58010

Merged
merged 8 commits into from
Feb 25, 2020
Merged

Add migration support to the event log #58010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cb86e22
Initial work
mikecote Feb 19, 2020
c2b83ca
Add missing file
mikecote Feb 19, 2020
dbc3111
Add tests where missing
mikecote Feb 19, 2020
8bca6a9
Merge branch 'master' of github.com:elastic/kibana into event_log/mig…
mikecote Feb 20, 2020
4d3d4a7
Merge branch 'master' of github.com:elastic/kibana into event_log/mig…
mikecote Feb 21, 2020
0d5bde1
Add kibana version to esNames
mikecote Feb 21, 2020
72d9bb1
Merge branch 'master' of github.com:elastic/kibana into event_log/mig…
mikecote Feb 24, 2020
c7209d0
Share ILM policy across versions
mikecote Feb 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ describe('createIndex', () => {
await clusterClientAdapter.createIndex('foo');
expect(clusterClient.callAsInternalUser).toHaveBeenCalledWith('indices.create', {
index: 'foo',
body: {},
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why we added the extra body here. I was frankly curious I could create an index WITHOUT a document to begin with, but I'm wondering if this change was deliberate, and if so, why was it done.

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 is to set additional settings when creating the initial index. I need this to setup the alias when creating the initial index now that it's not done with the index template anymore.

Reference: https://github.com/elastic/kibana/pull/58010/files#diff-88aee8214a0ea2006f946be367b1d715R66

});
});

Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,12 @@ export class ClusterClientAdapter {
return result as boolean;
}

public async createIndex(name: string): Promise<void> {
public async createIndex(name: string, body: any = {}): Promise<void> {
try {
await this.callEs('indices.create', { index: name });
await this.callEs('indices.create', {
index: name,
body,
});
} catch (err) {
if (err.body?.error?.type !== 'resource_already_exists_exception') {
throw new Error(`error creating initial index: ${err.message}`);
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/event_log/server/es/documents.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ describe('getIndexTemplate()', () => {

test('returns the correct details of the index template', () => {
const indexTemplate = getIndexTemplate(esNames);
expect(indexTemplate.index_patterns).toEqual([esNames.indexPattern]);
expect(indexTemplate.aliases[esNames.alias]).toEqual({});
expect(indexTemplate.index_patterns).toEqual([esNames.indexPatternWithVersion]);
expect(indexTemplate.settings.number_of_shards).toBeGreaterThanOrEqual(0);
expect(indexTemplate.settings.number_of_replicas).toBeGreaterThanOrEqual(0);
expect(indexTemplate.settings['index.lifecycle.name']).toBe(esNames.ilmPolicy);
Expand Down
5 changes: 1 addition & 4 deletions x-pack/plugins/event_log/server/es/documents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ import mappings from '../../generated/mappings.json';
// returns the body of an index template used in an ES indices.putTemplate call
export function getIndexTemplate(esNames: EsNames) {
const indexTemplateBody: any = {
index_patterns: [esNames.indexPattern],
aliases: {
Copy link
Member

Choose a reason for hiding this comment

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

I guess the aliases aren't really needed here, since it's already set up in the index.lifecycle.rollover_alias property below this?

I guess my only worry is that if it rolls over for some other reason, that didn't take the that ilm property into account, the aliases could get into some funky state. Not sure if it even CAN roll over for some other reason. :-).

One of the nice things about this version, is that if a rando user creates an index matching our template, it WON'T get aliased in for free, which is likely what we want to happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I recall, with the alias definition here, there would be ambiguity finding the write index after a second one is created. After doing some research, it was recommended to only setup is_write_index on the initial index and ILM would then handle the changeover automatically on rollover.

Reference: https://discuss.elastic.co/t/index-lifecycle-management-does-not-point-to-index-error/211513/2

[esNames.alias]: {},
},
index_patterns: [esNames.indexPatternWithVersion],
settings: {
number_of_shards: 1,
number_of_replicas: 1,
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/event_log/server/es/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ class EsInitializationSteps {
async createInitialIndexIfNotExists(): Promise<void> {
const exists = await this.esContext.esAdapter.doesAliasExist(this.esContext.esNames.alias);
if (!exists) {
await this.esContext.esAdapter.createIndex(this.esContext.esNames.initialIndex);
await this.esContext.esAdapter.createIndex(this.esContext.esNames.initialIndex, {
aliases: {
[this.esContext.esNames.alias]: {
is_write_index: true,
},
},
});
}
}
}
7 changes: 4 additions & 3 deletions x-pack/plugins/event_log/server/es/names.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import { EsNames } from './names';
const createNamesMock = () => {
const mock: jest.Mocked<EsNames> = {
base: '.kibana',
alias: '.kibana-event-log',
alias: '.kibana-event-log-8.0.0',
ilmPolicy: '.kibana-event-log-policy',
indexPattern: '.kibana-event-log-*',
initialIndex: '.kibana-event-log-000001',
indexTemplate: '.kibana-event-log-template',
indexPatternWithVersion: '.kibana-event-log-8.0.0-*',
initialIndex: '.kibana-event-log-8.0.0-000001',
indexTemplate: '.kibana-event-log-8.0.0-template',
};
return mock;
};
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/event_log/server/es/names.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,21 @@

import { getEsNames } from './names';

jest.mock('../lib/../../../../package.json', () => ({
version: '1.2.3',
}));

describe('getEsNames()', () => {
test('works as expected', () => {
const base = 'XYZ';
const version = '1.2.3';
const esNames = getEsNames(base);
expect(esNames.base).toEqual(base);
expect(esNames.alias).toEqual(`${base}-event-log`);
expect(esNames.alias).toEqual(`${base}-event-log-${version}`);
expect(esNames.ilmPolicy).toEqual(`${base}-event-log-policy`);
expect(esNames.indexPattern).toEqual(`${base}-event-log-*`);
expect(esNames.initialIndex).toEqual(`${base}-event-log-000001`);
expect(esNames.indexTemplate).toEqual(`${base}-event-log-template`);
expect(esNames.indexPatternWithVersion).toEqual(`${base}-event-log-${version}-*`);
expect(esNames.initialIndex).toEqual(`${base}-event-log-${version}-000001`);
expect(esNames.indexTemplate).toEqual(`${base}-event-log-${version}-template`);
});
});
14 changes: 10 additions & 4 deletions x-pack/plugins/event_log/server/es/names.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,31 @@
* you may not use this file except in compliance with the Elastic License.
*/

const EVENT_LOG_NAME_SUFFIX = '-event-log';
import xPackage from '../../../../package.json';

const EVENT_LOG_NAME_SUFFIX = `-event-log`;
const EVENT_LOG_VERSION_SUFFIX = `-${xPackage.version}`;

export interface EsNames {
base: string;
alias: string;
ilmPolicy: string;
indexPattern: string;
indexPatternWithVersion: string;
initialIndex: string;
indexTemplate: string;
}

export function getEsNames(baseName: string): EsNames {
const eventLogName = `${baseName}${EVENT_LOG_NAME_SUFFIX}`;
const eventLogNameWithVersion = `${eventLogName}${EVENT_LOG_VERSION_SUFFIX}`;
return {
base: baseName,
alias: eventLogName,
alias: eventLogNameWithVersion,
ilmPolicy: `${eventLogName}-policy`,
indexPattern: `${eventLogName}-*`,
initialIndex: `${eventLogName}-000001`,
indexTemplate: `${eventLogName}-template`,
indexPatternWithVersion: `${eventLogNameWithVersion}-*`,
initialIndex: `${eventLogNameWithVersion}-000001`,
indexTemplate: `${eventLogNameWithVersion}-template`,
};
}