From ff3a600d5eca300c769f67ce144364b5de3a2409 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 18 Dec 2024 14:14:57 +0100 Subject: [PATCH] [UA] Remove noisey warn when deleting from `.tasks` (#204720) ## Summary Reduce the number of `warn` logs reindexing will create due to failed attempts to delete from `.tasks`. In this PR we only log for responses that do not have status code 403 or 401. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../lib/reindexing/reindex_service.test.ts | 34 +++++++++++++++---- .../server/lib/reindexing/reindex_service.ts | 17 ++++------ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts index e160ac37a9e34..44aed05b2c82a 100644 --- a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts +++ b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts @@ -26,7 +26,6 @@ import { esIndicesStateCheck } from '../es_indices_state_check'; import { versionService } from '../version'; import { ReindexService, reindexServiceFactory } from './reindex_service'; -import { error } from './error'; const asApiResponse = (body: T): TransportResult => ({ @@ -638,12 +637,7 @@ describe('reindexService', () => { index: '.tasks', id: 'xyz', }); - expect(log.warn).toHaveBeenCalledTimes(1); - expect(log.warn).toHaveBeenCalledWith( - error.reindexTaskCannotBeDeleted( - `Could not delete reindexing task xyz, got response "!?"` - ) - ); + expect(log.warn).toHaveBeenCalledTimes(0); // Do not log anything in this case }); it('does not throw if task doc deletion throws', async () => { @@ -672,6 +666,32 @@ describe('reindexService', () => { expect(log.warn).toHaveBeenCalledTimes(1); expect(log.warn).toHaveBeenCalledWith(new Error('FAILED!')); }); + + it.each([401, 403])( + 'does not throw if task doc deletion throws AND does not log due to missing access permission: %d', + async (statusCode) => { + clusterClient.asCurrentUser.tasks.get.mockResponseOnce({ + completed: true, + // @ts-expect-error not full interface + task: { status: { created: 100, total: 100 } }, + }); + + clusterClient.asCurrentUser.count.mockResponseOnce( + // @ts-expect-error not full interface + { + count: 100, + } + ); + const e = new Error(); + Object.defineProperty(e, 'statusCode', { value: statusCode }); + clusterClient.asCurrentUser.delete.mockRejectedValue(e); + + const updatedOp = await service.processNextStep(reindexOp); + expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted); + expect(updatedOp.attributes.reindexTaskPercComplete).toEqual(1); + expect(log.warn).toHaveBeenCalledTimes(0); + } + ); }); describe('reindex task is cancelled', () => { diff --git a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts index ce1f19f6babb5..e55b4fce2e4d7 100644 --- a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts +++ b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts @@ -309,20 +309,17 @@ export const reindexServiceFactory = ( } try { - // Delete the task from ES .tasks index - const deleteTaskResp = await esClient.delete({ + // Best effort, delete the task from ES .tasks index... + await esClient.delete({ index: '.tasks', id: taskId, }); - if (deleteTaskResp.result !== 'deleted') { - log.warn( - error.reindexTaskCannotBeDeleted( - `Could not delete reindexing task ${taskId}, got response "${deleteTaskResp.result}"` - ) - ); - } } catch (e) { - log.warn(e); + // We explicitly ignore authz related error codes bc we expect this to be + // very common when deleting from .tasks + if (e?.statusCode !== 401 && e?.statusCode !== 403) { + log.warn(e); + } } return reindexOp;