add test for automatic index creation failure recovery

elastic · Jun 24, 2020 · 29bf6a7 · 29bf6a7
1 parent ef79c04
commit 29bf6a7
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 16 deletions.
diff --git a/x-pack/plugins/reporting/server/lib/store/store.test.ts b/x-pack/plugins/reporting/server/lib/store/store.test.ts
@@ -107,10 +107,37 @@ describe('ReportingStore', () => {
       ).rejects.toMatchInlineSnapshot(`[Error: error]`);
     });
 
+    /* Creating the index will fail, if there were multiple jobs staged in
+     * parallel and creation completed from another Kibana instance.  Only the
+     * first request in line can successfully create it.
+     * In spite of that race condition, adding the new job in Elasticsearch is
+     * fine.
+     */
+    it('ignores index creation error if the index already exists and continues adding the report', async () => {
+      // setup
+      callClusterStub.withArgs('indices.exists').resolves(false);
+      callClusterStub.withArgs('indices.create').rejects(new Error('error'));
+
+      const store = new ReportingStore(mockCore, mockLogger);
+      const reportType = 'unknowntype';
+      const reportPayload = {};
+      const reportOptions = {
+        timeout: 10000,
+        created_by: 'created_by_string',
+        browser_type: 'browser_type_string',
+        max_attempts: 1,
+      };
+      await expect(
+        store.addReport(reportType, reportPayload, reportOptions)
+      ).rejects.toMatchInlineSnapshot(`[Error: error]`);
+    });
+
     it('skips creating the index if already exists', async () => {
       // setup
-      callClusterStub.withArgs('indices.exists').resolves(true);
-      callClusterStub.withArgs('indices.create').rejects(new Error('error')); // will not be triggered
+      callClusterStub.withArgs('indices.exists').resolves(false);
+      callClusterStub
+        .withArgs('indices.create')
+        .rejects(new Error('resource_already_exists_exception')); // will be triggered but ignored
 
       const store = new ReportingStore(mockCore, mockLogger);
       const reportType = 'unknowntype';

diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts
@@ -87,21 +87,12 @@ export class ReportingStore {
             body,
           })
           .then(() => true)
-          .catch((err) => {
-            /* FIXME creating the index will fail if there were multiple jobs staged in parallel.
-             * Each staged job checks `client.indices.exists` and could each get `false` as a response.
-             * Only the first job in line can successfully create it though.
-             * The problem might only happen in automated tests, where the indices are deleted after each test run.
-             * This catch block is in place to not fail a job if the job runner hits this race condition.
-             * Unfortunately we don't have a logger in scope to log a warning.
-             */
-            const isIndexExistsError =
-              err &&
-              err.body &&
-              err.body.error &&
-              err.body.error.type === 'resource_already_exists_exception';
+          .catch((err: Error) => {
+            const isIndexExistsError = err.message.match(/resource_already_exists_exception/);
             if (isIndexExistsError) {
-              return true;
+              // Do not fail a job if the job runner hits the race condition.
+              this.logger.warn(`Automatic index creation failed: index already exists: ${err}`);
+              return;
             }
 
             throw err;