Skip to content

Commit 3ab9da8

Browse files
jloleysensbenakansara
authored andcommitted
[UA] Remove noisey warn when deleting from .tasks (elastic#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
1 parent 12d16f3 commit 3ab9da8

File tree

2 files changed

+34
-17
lines changed

2 files changed

+34
-17
lines changed

x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import { esIndicesStateCheck } from '../es_indices_state_check';
2626
import { versionService } from '../version';
2727

2828
import { ReindexService, reindexServiceFactory } from './reindex_service';
29-
import { error } from './error';
3029

3130
const asApiResponse = <T>(body: T): TransportResult<T> =>
3231
({
@@ -638,12 +637,7 @@ describe('reindexService', () => {
638637
index: '.tasks',
639638
id: 'xyz',
640639
});
641-
expect(log.warn).toHaveBeenCalledTimes(1);
642-
expect(log.warn).toHaveBeenCalledWith(
643-
error.reindexTaskCannotBeDeleted(
644-
`Could not delete reindexing task xyz, got response "!?"`
645-
)
646-
);
640+
expect(log.warn).toHaveBeenCalledTimes(0); // Do not log anything in this case
647641
});
648642

649643
it('does not throw if task doc deletion throws', async () => {
@@ -672,6 +666,32 @@ describe('reindexService', () => {
672666
expect(log.warn).toHaveBeenCalledTimes(1);
673667
expect(log.warn).toHaveBeenCalledWith(new Error('FAILED!'));
674668
});
669+
670+
it.each([401, 403])(
671+
'does not throw if task doc deletion throws AND does not log due to missing access permission: %d',
672+
async (statusCode) => {
673+
clusterClient.asCurrentUser.tasks.get.mockResponseOnce({
674+
completed: true,
675+
// @ts-expect-error not full interface
676+
task: { status: { created: 100, total: 100 } },
677+
});
678+
679+
clusterClient.asCurrentUser.count.mockResponseOnce(
680+
// @ts-expect-error not full interface
681+
{
682+
count: 100,
683+
}
684+
);
685+
const e = new Error();
686+
Object.defineProperty(e, 'statusCode', { value: statusCode });
687+
clusterClient.asCurrentUser.delete.mockRejectedValue(e);
688+
689+
const updatedOp = await service.processNextStep(reindexOp);
690+
expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
691+
expect(updatedOp.attributes.reindexTaskPercComplete).toEqual(1);
692+
expect(log.warn).toHaveBeenCalledTimes(0);
693+
}
694+
);
675695
});
676696

677697
describe('reindex task is cancelled', () => {

x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -309,20 +309,17 @@ export const reindexServiceFactory = (
309309
}
310310

311311
try {
312-
// Delete the task from ES .tasks index
313-
const deleteTaskResp = await esClient.delete({
312+
// Best effort, delete the task from ES .tasks index...
313+
await esClient.delete({
314314
index: '.tasks',
315315
id: taskId,
316316
});
317-
if (deleteTaskResp.result !== 'deleted') {
318-
log.warn(
319-
error.reindexTaskCannotBeDeleted(
320-
`Could not delete reindexing task ${taskId}, got response "${deleteTaskResp.result}"`
321-
)
322-
);
323-
}
324317
} catch (e) {
325-
log.warn(e);
318+
// We explicitly ignore authz related error codes bc we expect this to be
319+
// very common when deleting from .tasks
320+
if (e?.statusCode !== 401 && e?.statusCode !== 403) {
321+
log.warn(e);
322+
}
326323
}
327324

328325
return reindexOp;

0 commit comments

Comments
 (0)