Skip to content

Commit

Permalink
[8.2] migrations incorrectly detects cluster routing allocation setti…
Browse files Browse the repository at this point in the history 
…ng as incompatible (#131712) (#132284)

* migrations incorrectly detects cluster routing allocation setting as incompatible (#131712)

* Add reproducing test case

* Fix and add integration test

* Transient settings should take preference

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 638bfbe)

# Conflicts:
#	src/core/server/saved_objects/migrations/actions/initialize_action.ts
#	src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts

* Fix merge conflict
  • Loading branch information
@rudolf
rudolf committed May 18, 2022
1 parent 7ede480 commit 9a3c3f1
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 83 deletions.
103 changes: 94 additions & 9 deletions src/core/server/saved_objects/migrations/actions/initialize_action.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import * as Either from 'fp-ts/lib/Either';
import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
import { errors as EsErrors } from '@elastic/elasticsearch';
jest.mock('./catch_retryable_es_client_errors');
Expand All @@ -16,16 +17,16 @@ describe('initAction', () => {
beforeEach(() => {
jest.clearAllMocks();
});
const retryableError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 503,
body: { error: { type: 'es_type', reason: 'es_reason' } },
})
);
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(retryableError)
);
it('calls catchRetryableEsClientErrors when the promise rejects', async () => {
const retryableError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 503,
body: { error: { type: 'es_type', reason: 'es_reason' } },
})
);
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(retryableError)
);
const task = initAction({ client, indices: ['my_index'] });
try {
await task();
Expand All @@ -34,4 +35,88 @@ describe('initAction', () => {
}
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('resolves right when persistent and transient cluster settings are compatible', async () => {
const clusterSettingsResponse = {
transient: { 'cluster.routing.allocation.enable': 'all' },
persistent: { 'cluster.routing.allocation.enable': 'all' },
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isRight(result)).toEqual(true);
});
it('resolves right when persistent and transient cluster settings are undefined', async () => {
const clusterSettingsResponse = {
transient: {},
persistent: {},
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isRight(result)).toEqual(true);
});
it('resolves right when persistent cluster settings are compatible', async () => {
const clusterSettingsResponse = {
transient: {},
persistent: { 'cluster.routing.allocation.enable': 'all' },
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isRight(result)).toEqual(true);
});
it('resolves right when transient cluster settings are compatible', async () => {
const clusterSettingsResponse = {
transient: { 'cluster.routing.allocation.enable': 'all' },
persistent: {},
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isRight(result)).toEqual(true);
});
it('resolves right when valid transient settings, incompatible persistent settings', async () => {
const clusterSettingsResponse = {
transient: { 'cluster.routing.allocation.enable': 'all' },
persistent: { 'cluster.routing.allocation.enable': 'primaries' },
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isRight(result)).toEqual(true);
});
it('resolves left when valid persistent settings, incompatible transient settings', async () => {
const clusterSettingsResponse = {
transient: { 'cluster.routing.allocation.enable': 'primaries' },
persistent: { 'cluster.routing.allocation.enable': 'alls' },
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isLeft(result)).toEqual(true);
});
it('resolves left when transient cluster settings are incompatible', async () => {
const clusterSettingsResponse = {
transient: { 'cluster.routing.allocation.enable': 'none' },
persistent: { 'cluster.routing.allocation.enable': 'all' },
};
const client = elasticsearchClientMock.createInternalClient(
new Promise((res) => res(clusterSettingsResponse))
);
const task = initAction({ client, indices: ['my_index'] });
const result = await task();
expect(Either.isLeft(result)).toEqual(true);
});
});
19 changes: 11 additions & 8 deletions src/core/server/saved_objects/migrations/actions/initialize_action.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,20 @@ export const checkClusterRoutingAllocationEnabledTask =
flat_settings: true,
})
.then((settings) => {
const clusterRoutingAllocations: string[] =
// transient settings take preference over persistent settings
const clusterRoutingAllocation =
settings?.transient?.[routingAllocationEnable] ??
settings?.persistent?.[routingAllocationEnable] ??
[];
settings?.persistent?.[routingAllocationEnable];

const clusterRoutingAllocationEnabled =
[...clusterRoutingAllocations].length === 0 ||
[...clusterRoutingAllocations].every((s: string) => s === 'all'); // if set, only allow 'all'
const clusterRoutingAllocationEnabledIsAll =
clusterRoutingAllocation === undefined || clusterRoutingAllocation === 'all';

if (!clusterRoutingAllocationEnabled) {
return Either.left({ type: 'unsupported_cluster_routing_allocation' as const });
if (!clusterRoutingAllocationEnabledIsAll) {
return Either.left({
type: 'unsupported_cluster_routing_allocation' as const,
message:
'[unsupported_cluster_routing_allocation] The elasticsearch cluster has cluster routing allocation incorrectly set for migrations to continue.',
});
} else {
return Either.right({});
}
Expand Down
151 changes: 85 additions & 66 deletions src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { ElasticsearchClient } from '../../../../';
import { ElasticsearchClient } from '../../../..';
import * as kbnTestServer from '../../../../../test_helpers/kbn_server';
import { SavedObjectsRawDoc } from '../../../serialization';
import {
Expand Down Expand Up @@ -35,7 +35,7 @@ import {
transformDocs,
waitForIndexStatusYellow,
initAction,
} from '../../actions';
} from '..';
import * as Either from 'fp-ts/lib/Either';
import * as Option from 'fp-ts/lib/Option';
import { errors } from '@elastic/elasticsearch';
Expand Down Expand Up @@ -116,7 +116,7 @@ describe('migration actions', () => {
await client.cluster.putSettings({
body: {
persistent: {
// Remove persistent test settings
// Reset persistent test settings
cluster: { routing: { allocation: { enable: null } } },
},
},
Expand All @@ -126,11 +126,11 @@ describe('migration actions', () => {
expect.assertions(1);
const task = initAction({ client, indices: ['no_such_index'] });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": Object {},
}
`);
Object {
"_tag": "Right",
"right": Object {},
}
`);
});
it('resolves right record with found indices', async () => {
expect.assertions(1);
Expand All @@ -149,7 +149,7 @@ describe('migration actions', () => {
})
);
});
it('resolves left with cluster routing allocation disabled', async () => {
it('resolves left when cluster.routing.allocation.enabled is incompatible', async () => {
expect.assertions(3);
await client.cluster.putSettings({
body: {
Expand All @@ -164,13 +164,14 @@ describe('migration actions', () => {
indices: ['existing_index_with_docs'],
});
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"type": "unsupported_cluster_routing_allocation",
},
}
`);
Object {
"_tag": "Left",
"left": Object {
"message": "[unsupported_cluster_routing_allocation] The elasticsearch cluster has cluster routing allocation incorrectly set for migrations to continue.",
"type": "unsupported_cluster_routing_allocation",
},
}
`);
await client.cluster.putSettings({
body: {
persistent: {
Expand All @@ -184,13 +185,14 @@ describe('migration actions', () => {
indices: ['existing_index_with_docs'],
});
await expect(task2()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"type": "unsupported_cluster_routing_allocation",
},
}
`);
Object {
"_tag": "Left",
"left": Object {
"message": "[unsupported_cluster_routing_allocation] The elasticsearch cluster has cluster routing allocation incorrectly set for migrations to continue.",
"type": "unsupported_cluster_routing_allocation",
},
}
`);
await client.cluster.putSettings({
body: {
persistent: {
Expand All @@ -204,13 +206,30 @@ describe('migration actions', () => {
indices: ['existing_index_with_docs'],
});
await expect(task3()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"type": "unsupported_cluster_routing_allocation",
},
}
`);
Object {
"_tag": "Left",
"left": Object {
"message": "[unsupported_cluster_routing_allocation] The elasticsearch cluster has cluster routing allocation incorrectly set for migrations to continue.",
"type": "unsupported_cluster_routing_allocation",
},
}
`);
});
it('resolves right when cluster.routing.allocation.enabled=all', async () => {
expect.assertions(1);
await client.cluster.putSettings({
body: {
persistent: {
cluster: { routing: { allocation: { enable: 'all' } } },
},
},
});
const task = initAction({
client,
indices: ['existing_index_with_docs'],
});
const result = await task();
expect(Either.isRight(result)).toBe(true);
});
});

Expand Down Expand Up @@ -268,14 +287,14 @@ describe('migration actions', () => {
expect.assertions(1);
const task = setWriteBlock({ client, index: 'no_such_index' });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"index": "no_such_index",
"type": "index_not_found_exception",
},
}
`);
Object {
"_tag": "Left",
"left": Object {
"index": "no_such_index",
"type": "index_not_found_exception",
},
}
`);
});
});

Expand All @@ -297,21 +316,21 @@ describe('migration actions', () => {
expect.assertions(1);
const task = removeWriteBlock({ client, index: 'existing_index_with_write_block_2' });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": "remove_write_block_succeeded",
}
`);
Object {
"_tag": "Right",
"right": "remove_write_block_succeeded",
}
`);
});
it('resolves right if successful when an index does not have a write block', async () => {
expect.assertions(1);
const task = removeWriteBlock({ client, index: 'existing_index_without_write_block_2' });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": "remove_write_block_succeeded",
}
`);
Object {
"_tag": "Right",
"right": "remove_write_block_succeeded",
}
`);
});
it('rejects if there is a non-retryable error', async () => {
expect.assertions(1);
Expand Down Expand Up @@ -384,13 +403,13 @@ describe('migration actions', () => {
});
expect.assertions(1);
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": Object {
"acknowledged": true,
"shardsAcknowledged": true,
},
}
Object {
"_tag": "Right",
"right": Object {
"acknowledged": true,
"shardsAcknowledged": true,
},
}
`);
});
it('resolves right after waiting for index status to be yellow if clone target already existed', async () => {
Expand Down Expand Up @@ -450,13 +469,13 @@ describe('migration actions', () => {
expect.assertions(1);
const task = cloneIndex({ client, source: 'no_such_index', target: 'clone_target_3' });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"index": "no_such_index",
"type": "index_not_found_exception",
},
}
Object {
"_tag": "Left",
"left": Object {
"index": "no_such_index",
"type": "index_not_found_exception",
},
}
`);
});
it('resolves left with a retryable_es_client_error if clone target already exists but takes longer than the specified timeout before turning yellow', async () => {
Expand Down Expand Up @@ -579,10 +598,10 @@ describe('migration actions', () => {
})()) as Either.Right<ReindexResponse>;
const task = waitForReindexTask({ client, taskId: res.right.taskId, timeout: '10s' });
await expect(task()).resolves.toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": "reindex_succeeded",
}
Object {
"_tag": "Right",
"right": "reindex_succeeded",
}
`);

const results = (
Expand Down

0 comments on commit 9a3c3f1

Please sign in to comment.