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

Add migration support to the event log #58010

Merged
merged 8 commits into from
Feb 25, 2020
Prev Previous commit
Next Next commit
Add tests where missing
mikecote committed Feb 19, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit dbc3111763a9771533f989d860128dd4d0e7ff30
38 changes: 38 additions & 0 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts
Original file line number Diff line number Diff line change
@@ -140,7 +140,28 @@ describe('createIndexTemplate', () => {
});
});

describe('updateIndexTemplate', () => {
test('should call cluster with given template', async () => {
await clusterClientAdapter.updateIndexTemplate('foo', { args: true });
expect(clusterClient.callAsInternalUser).toHaveBeenCalledWith('indices.putTemplate', {
name: 'foo',
body: { args: true },
});
});

test('should throw error when call cluster throws', async () => {
clusterClient.callAsInternalUser.mockRejectedValue(new Error('Fail'));
await expect(
clusterClientAdapter.updateIndexTemplate('foo', { args: true })
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});

describe('getIndexTemplate', () => {
beforeEach(() => {
clusterClient.callAsInternalUser.mockResolvedValue({});
});

test('should call cluster with given name', async () => {
await clusterClientAdapter.getIndexTemplate('foo');
expect(clusterClient.callAsInternalUser).toHaveBeenCalledWith('indices.getTemplate', {
@@ -189,6 +210,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

});
});

@@ -210,3 +232,19 @@ describe('createIndex', () => {
await clusterClientAdapter.createIndex('foo');
});
});

describe('rolloverIndex', () => {
test('should call cluster with given body', async () => {
await clusterClientAdapter.rolloverIndex({ args: true });
expect(clusterClient.callAsInternalUser).toHaveBeenCalledWith('indices.rollover', {
args: true,
});
});

test('should throw error when call cluster throws', async () => {
clusterClient.callAsInternalUser.mockRejectedValue(new Error('Fail'));
await expect(
clusterClientAdapter.rolloverIndex({ args: true })
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});
2 changes: 1 addition & 1 deletion x-pack/plugins/event_log/server/es/documents.test.ts
Original file line number Diff line number Diff line change
@@ -23,11 +23,11 @@ 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.settings.number_of_shards).toBeGreaterThanOrEqual(0);
expect(indexTemplate.settings.number_of_replicas).toBeGreaterThanOrEqual(0);
expect(indexTemplate.settings['index.lifecycle.name']).toBe(esNames.ilmPolicy);
expect(indexTemplate.settings['index.lifecycle.rollover_alias']).toBe(esNames.alias);
expect(indexTemplate.mappings).toMatchObject({});
expect(indexTemplate.version).toBeGreaterThanOrEqual(0);
});
});
37 changes: 37 additions & 0 deletions x-pack/plugins/event_log/server/es/init.test.ts
Original file line number Diff line number Diff line change
@@ -7,6 +7,10 @@
import { contextMock } from './context.mock';
import { initializeEs } from './init';

jest.mock('../lib/get_current_version_as_integer', () => ({
getCurrentVersionAsInteger: () => 1,
}));

describe('initializeEs', () => {
let esContext = contextMock.create();

@@ -61,4 +65,37 @@ describe('initializeEs', () => {
expect(esContext.esAdapter.doesAliasExist).toHaveBeenCalled();
expect(esContext.esAdapter.createIndex).not.toHaveBeenCalled();
});

test('should migrate index template when kibana version is more recent', async () => {
esContext.esAdapter.getIndexTemplate.mockResolvedValue({
version: 0,
});

await initializeEs(esContext);
expect(esContext.esAdapter.getIndexTemplate).toHaveBeenCalled();
expect(esContext.esAdapter.updateIndexTemplate).toHaveBeenCalled();
expect(esContext.esAdapter.rolloverIndex).toHaveBeenCalled();
});

test(`shouldn't migrate index template when kibana version is the same`, async () => {
esContext.esAdapter.getIndexTemplate.mockResolvedValue({
version: 1,
});

await initializeEs(esContext);
expect(esContext.esAdapter.getIndexTemplate).toHaveBeenCalled();
expect(esContext.esAdapter.updateIndexTemplate).not.toHaveBeenCalled();
expect(esContext.esAdapter.rolloverIndex).not.toHaveBeenCalled();
});

test('should log error if template version in Elasticsearch is more recent', async () => {
esContext.esAdapter.getIndexTemplate.mockResolvedValue({
version: 2,
});

await initializeEs(esContext);
expect(esContext.logger.error).toHaveBeenLastCalledWith(
'error initializing elasticsearch resources: Index template belongs to a more recent version of Kibana (2)'
);
});
});
7 changes: 6 additions & 1 deletion x-pack/plugins/event_log/server/es/init.ts
Original file line number Diff line number Diff line change
@@ -74,10 +74,11 @@ class EsInitializationSteps {
}

async migrateIndexTemplate() {
const updatedTemplateBody = getIndexTemplate(this.esContext.esNames);
const existingTemplate = await this.esContext.esAdapter.getIndexTemplate(
this.esContext.esNames.indexTemplate
);
const updatedTemplateBody = getIndexTemplate(this.esContext.esNames);

if (updatedTemplateBody.version > existingTemplate.version) {
await this.esContext.esAdapter.updateIndexTemplate(
this.esContext.esNames.indexTemplate,
@@ -91,6 +92,10 @@ class EsInitializationSteps {
},
},
});
} else if (updatedTemplateBody.version < existingTemplate.version) {
throw new Error(
`Index template belongs to a more recent version of Kibana (${existingTemplate.version})`
);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

describe('getCurrentVersionAsInteger', () => {
beforeEach(() => jest.resetModules());

test('should parse 8.0.0', () => {
jest.mock('../../../../package.json', () => ({
version: '8.0.0',
}));
const { getCurrentVersionAsInteger } = jest.requireActual('./get_current_version_as_integer');
expect(getCurrentVersionAsInteger()).toEqual(80000);
});

test('should parse 8.1.0', () => {
jest.mock('../../../../package.json', () => ({
version: '8.1.0',
}));
const { getCurrentVersionAsInteger } = jest.requireActual('./get_current_version_as_integer');
expect(getCurrentVersionAsInteger()).toEqual(80100);
});

test('should parse 8.1.2', () => {
jest.mock('../../../../package.json', () => ({
version: '8.1.2',
}));
const { getCurrentVersionAsInteger } = jest.requireActual('./get_current_version_as_integer');
expect(getCurrentVersionAsInteger()).toEqual(80102);
});

test('should parse v8.1.2', () => {
jest.mock('../../../../package.json', () => ({
version: 'v8.1.2',
}));
const { getCurrentVersionAsInteger } = jest.requireActual('./get_current_version_as_integer');
expect(getCurrentVersionAsInteger()).toEqual(80102);
});
});