From 9a3c3f13c817f18bc57225f28a4a5d5a6b4043e9 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 18 May 2022 12:54:41 +0200 Subject: [PATCH] [8.2] migrations incorrectly detects cluster routing allocation setting 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 638bfbee3d136ca73458c8ae0bb815893f35ef5f) # 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 --- .../actions/initialize_action.test.ts | 103 ++++++++++-- .../migrations/actions/initialize_action.ts | 19 ++- .../actions/integration_tests/actions.test.ts | 151 ++++++++++-------- 3 files changed, 190 insertions(+), 83 deletions(-) diff --git a/src/core/server/saved_objects/migrations/actions/initialize_action.test.ts b/src/core/server/saved_objects/migrations/actions/initialize_action.test.ts index 7c75470b890aa..5d831a5bb8f78 100644 --- a/src/core/server/saved_objects/migrations/actions/initialize_action.test.ts +++ b/src/core/server/saved_objects/migrations/actions/initialize_action.test.ts @@ -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'); @@ -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(); @@ -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); + }); }); diff --git a/src/core/server/saved_objects/migrations/actions/initialize_action.ts b/src/core/server/saved_objects/migrations/actions/initialize_action.ts index 73502382c9ca0..5630382816fba 100644 --- a/src/core/server/saved_objects/migrations/actions/initialize_action.ts +++ b/src/core/server/saved_objects/migrations/actions/initialize_action.ts @@ -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({}); } diff --git a/src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts b/src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts index fb14a0c8b3a01..7d00f5291192f 100644 --- a/src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts +++ b/src/core/server/saved_objects/migrations/actions/integration_tests/actions.test.ts @@ -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 { @@ -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'; @@ -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 } } }, }, }, @@ -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); @@ -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: { @@ -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: { @@ -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: { @@ -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); }); }); @@ -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", + }, + } + `); }); }); @@ -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); @@ -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 () => { @@ -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 () => { @@ -579,10 +598,10 @@ describe('migration actions', () => { })()) as Either.Right; 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 = (