[Upgrade Assistant] Improve error messages for GET /api/upgrade_assis…

…tant/reindex/<index> (#112961)
elastic · Sep 29, 2021 · ade05d9 · ade05d9
1 parent 51f6b13
commit ade05d9
Show file tree

Hide file tree

Showing 11 changed files with 229 additions and 28 deletions.
diff --git a/.../upgrade_assistant/__jest__/client_integration/es_deprecations/es_deprecations.helpers.ts b/.../upgrade_assistant/__jest__/client_integration/es_deprecations/es_deprecations.helpers.ts
@@ -125,11 +125,22 @@ const createActions = (testBed: TestBed) => {
     },
   };
 
+  const reindexDeprecationFlyout = {
+    clickReindexButton: async () => {
+      await act(async () => {
+        find('startReindexingButton').simulate('click');
+      });
+
+      component.update();
+    },
+  };
+
   return {
     table,
     searchBar,
     pagination,
     mlDeprecationFlyout,
+    reindexDeprecationFlyout,
     indexSettingsDeprecationFlyout,
   };
 };

diff --git a/..._assistant/__jest__/client_integration/es_deprecations/reindex_deprecation_flyout.test.ts b/..._assistant/__jest__/client_integration/es_deprecations/reindex_deprecation_flyout.test.ts
@@ -47,4 +47,51 @@ describe('Reindex deprecation flyout', () => {
       `Reindex ${reindexDeprecation.index}`
     );
   });
+
+  it('renders error callout when reindex fails', async () => {
+    httpRequestsMockHelpers.setReindexStatusResponse({
+      reindexOp: null,
+      warnings: [],
+      indexGroup: null,
+      hasRequiredPrivileges: true,
+    });
+
+    await act(async () => {
+      testBed = await setupElasticsearchPage({ isReadOnlyMode: false });
+    });
+
+    testBed.component.update();
+
+    const { actions, exists } = testBed;
+
+    await actions.table.clickDeprecationRowAt('reindex', 0);
+
+    httpRequestsMockHelpers.setStartReindexingResponse(undefined, {
+      statusCode: 404,
+      message: 'no such index [test]',
+    });
+
+    await actions.reindexDeprecationFlyout.clickReindexButton();
+
+    expect(exists('reindexDetails.reindexingFailedCallout')).toBe(true);
+  });
+
+  it('renders error callout when fetch status fails', async () => {
+    httpRequestsMockHelpers.setReindexStatusResponse(undefined, {
+      statusCode: 404,
+      message: 'no such index [test]',
+    });
+
+    await act(async () => {
+      testBed = await setupElasticsearchPage({ isReadOnlyMode: false });
+    });
+
+    testBed.component.update();
+
+    const { actions, exists } = testBed;
+
+    await actions.table.clickDeprecationRowAt('reindex', 0);
+
+    expect(exists('reindexDetails.fetchFailedCallout')).toBe(true);
+  });
 });
diff --git a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts
@@ -115,6 +115,28 @@ const registerHttpRequestMockHelpers = (server: SinonFakeServer) => {
     ]);
   };
 
+  const setReindexStatusResponse = (response?: object, error?: ResponseError) => {
+    const status = error ? error.statusCode || 400 : 200;
+    const body = error ? error : response;
+
+    server.respondWith('GET', `${API_BASE_PATH}/reindex/:indexName`, [
+      status,
+      { 'Content-Type': 'application/json' },
+      JSON.stringify(body),
+    ]);
+  };
+
+  const setStartReindexingResponse = (response?: object, error?: ResponseError) => {
+    const status = error ? error.statusCode || 400 : 200;
+    const body = error ? error : response;
+
+    server.respondWith('POST', `${API_BASE_PATH}/reindex/:indexName`, [
+      status,
+      { 'Content-Type': 'application/json' },
+      JSON.stringify(body),
+    ]);
+  };
+
   const setDeleteMlSnapshotResponse = (response?: object, error?: ResponseError) => {
     const status = error ? error.statusCode || 400 : 200;
     const body = error ? error : response;
@@ -147,6 +169,8 @@ const registerHttpRequestMockHelpers = (server: SinonFakeServer) => {
     setDeleteMlSnapshotResponse,
     setUpgradeMlSnapshotStatusResponse,
     setLoadDeprecationLogsCountResponse,
+    setStartReindexingResponse,
+    setReindexStatusResponse,
     setLoadMlUpgradeModeResponse,
   };
 };

diff --git a/x-pack/plugins/upgrade_assistant/common/types.ts b/x-pack/plugins/upgrade_assistant/common/types.ts
@@ -31,6 +31,9 @@ export enum ReindexStatus {
   failed,
   paused,
   cancelled,
+  // Used by the UI to differentiate if there was a failure retrieving
+  // the status from the server API
+  fetchFailed,
 }
 
 export const REINDEX_OP_TYPE = 'upgrade-assistant-reindex-operation';

diff --git a/..._deprecations/deprecation_types/reindex/flyout/__snapshots__/checklist_step.test.tsx.snap b/..._deprecations/deprecation_types/reindex/flyout/__snapshots__/checklist_step.test.tsx.snap
diff --git a/...ation/components/es_deprecations/deprecation_types/reindex/flyout/checklist_step.test.tsx b/...ation/components/es_deprecations/deprecation_types/reindex/flyout/checklist_step.test.tsx
@@ -50,11 +50,35 @@ describe('ChecklistFlyout', () => {
     expect((wrapper.find('EuiButton').props() as any).isLoading).toBe(true);
   });
 
-  it('disables button if hasRequiredPrivileges is false', () => {
+  it('hides button if hasRequiredPrivileges is false', () => {
     const props = cloneDeep(defaultProps);
     props.reindexState.hasRequiredPrivileges = false;
     const wrapper = shallow(<ChecklistFlyoutStep {...props} />);
-    expect(wrapper.find('EuiButton').props().disabled).toBe(true);
+    expect(wrapper.exists('EuiButton')).toBe(false);
+  });
+
+  it('hides button if has error', () => {
+    const props = cloneDeep(defaultProps);
+    props.reindexState.status = ReindexStatus.fetchFailed;
+    props.reindexState.errorMessage = 'Index not found';
+    const wrapper = shallow(<ChecklistFlyoutStep {...props} />);
+    expect(wrapper.exists('EuiButton')).toBe(false);
+  });
+
+  it('shows get status error callout', () => {
+    const props = cloneDeep(defaultProps);
+    props.reindexState.status = ReindexStatus.fetchFailed;
+    props.reindexState.errorMessage = 'Index not found';
+    const wrapper = shallow(<ChecklistFlyoutStep {...props} />);
+    expect(wrapper.exists('[data-test-subj="fetchFailedCallout"]')).toBe(true);
+  });
+
+  it('shows reindexing callout', () => {
+    const props = cloneDeep(defaultProps);
+    props.reindexState.status = ReindexStatus.failed;
+    props.reindexState.errorMessage = 'Index not found';
+    const wrapper = shallow(<ChecklistFlyoutStep {...props} />);
+    expect(wrapper.exists('[data-test-subj="reindexingFailedCallout"]')).toBe(true);
   });
 
   it('calls startReindex when button is clicked', () => {

diff --git a/...pplication/components/es_deprecations/deprecation_types/reindex/flyout/checklist_step.tsx b/...pplication/components/es_deprecations/deprecation_types/reindex/flyout/checklist_step.tsx
@@ -76,6 +76,9 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
 }> = ({ closeFlyout, reindexState, startReindex, cancelReindex, renderGlobalCallouts }) => {
   const { loadingState, status, hasRequiredPrivileges } = reindexState;
   const loading = loadingState === LoadingState.Loading || status === ReindexStatus.inProgress;
+  const isCompleted = status === ReindexStatus.completed;
+  const hasFetchFailed = status === ReindexStatus.fetchFailed;
+  const hasReindexingFailed = status === ReindexStatus.failed;
 
   const onStartReindex = useCallback(() => {
     uiMetricService.trackUiMetric(METRIC_TYPE.CLICK, UIM_REINDEX_START_CLICK);
@@ -90,6 +93,46 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
   return (
     <Fragment>
       <EuiFlyoutBody>
+        {hasRequiredPrivileges === false && (
+          <Fragment>
+            <EuiSpacer />
+            <EuiCallOut
+              title={
+                <FormattedMessage
+                  id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.insufficientPrivilegeCallout.calloutTitle"
+                  defaultMessage="You do not have sufficient privileges to reindex this index"
+                />
+              }
+              color="danger"
+              iconType="alert"
+            />
+          </Fragment>
+        )}
+        {(hasFetchFailed || hasReindexingFailed) && (
+          <>
+            <EuiSpacer />
+            <EuiCallOut
+              color="danger"
+              iconType="alert"
+              data-test-subj={hasFetchFailed ? 'fetchFailedCallout' : 'reindexingFailedCallout'}
+              title={
+                hasFetchFailed ? (
+                  <FormattedMessage
+                    id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.fetchFailedCalloutTitle"
+                    defaultMessage="Reindex status not available"
+                  />
+                ) : (
+                  <FormattedMessage
+                    id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexingFailedCalloutTitle"
+                    defaultMessage="Reindexing error"
+                  />
+                )
+              }
+            >
+              {reindexState.errorMessage}
+            </EuiCallOut>
+          </>
+        )}
         {renderGlobalCallouts()}
         <EuiCallOut
           title={
@@ -116,21 +159,6 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
             />
           </p>
         </EuiCallOut>
-        {!hasRequiredPrivileges && (
-          <Fragment>
-            <EuiSpacer />
-            <EuiCallOut
-              title={
-                <FormattedMessage
-                  id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.insufficientPrivilegeCallout.calloutTitle"
-                  defaultMessage="You do not have sufficient privileges to reindex this index"
-                />
-              }
-              color="danger"
-              iconType="alert"
-            />
-          </Fragment>
-        )}
         <EuiSpacer />
         <EuiTitle size="xs">
           <h3>
@@ -152,7 +180,7 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
               />
             </EuiButtonEmpty>
           </EuiFlexItem>
-          {status !== ReindexStatus.completed && (
+          {!hasFetchFailed && !isCompleted && hasRequiredPrivileges && (
             <EuiFlexItem grow={false}>
               <EuiButton
                 fill
@@ -161,6 +189,7 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
                 onClick={onStartReindex}
                 isLoading={loading}
                 disabled={loading || !hasRequiredPrivileges}
+                data-test-subj="startReindexingButton"
               >
                 {buttonLabel(status)}
               </EuiButton>

diff --git a/...pplication/components/es_deprecations/deprecation_types/reindex/resolution_table_cell.tsx b/...pplication/components/es_deprecations/deprecation_types/reindex/resolution_table_cell.tsx
@@ -45,6 +45,12 @@ const i18nTexts = {
       defaultMessage: 'Reindex failed',
     }
   ),
+  reindexFetchFailedText: i18n.translate(
+    'xpack.upgradeAssistant.esDeprecations.reindex.reindexFetchFailedText',
+    {
+      defaultMessage: 'Reindex status not available',
+    }
+  ),
   reindexCanceledText: i18n.translate(
     'xpack.upgradeAssistant.esDeprecations.reindex.reindexCanceledText',
     {
@@ -119,6 +125,17 @@ export const ReindexResolutionCell: React.FunctionComponent = () => {
           </EuiFlexItem>
         </EuiFlexGroup>
       );
+    case ReindexStatus.fetchFailed:
+      return (
+        <EuiFlexGroup gutterSize="s" alignItems="center">
+          <EuiFlexItem grow={false}>
+            <EuiIcon type="alert" color="danger" />
+          </EuiFlexItem>
+          <EuiFlexItem grow={false}>
+            <EuiText size="s">{i18nTexts.reindexFetchFailedText}</EuiText>
+          </EuiFlexItem>
+        </EuiFlexGroup>
+      );
     case ReindexStatus.paused:
       return (
         <EuiFlexGroup gutterSize="s" alignItems="center">

diff --git a/...ic/application/components/es_deprecations/deprecation_types/reindex/use_reindex_state.tsx b/...ic/application/components/es_deprecations/deprecation_types/reindex/use_reindex_state.tsx
@@ -100,7 +100,8 @@ export const useReindexStatus = ({ indexName, api }: { indexName: string; api: A
       setReindexState({
         ...reindexState,
         loadingState: LoadingState.Error,
-        status: ReindexStatus.failed,
+        errorMessage: error.message.toString(),
+        status: ReindexStatus.fetchFailed,
       });
       return;
     }
@@ -137,6 +138,7 @@ export const useReindexStatus = ({ indexName, api }: { indexName: string; api: A
       setReindexState({
         ...reindexState,
         loadingState: LoadingState.Error,
+        errorMessage: error.message.toString(),
         status: ReindexStatus.failed,
       });
       return;

diff --git a/x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.test.ts b/x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.test.ts
@@ -11,6 +11,8 @@ import { licensingMock } from '../../../../licensing/server/mocks';
 import { securityMock } from '../../../../security/server/mocks';
 import { createMockRouter, MockRouter, routeHandlerContextMock } from '../__mocks__/routes.mock';
 import { createRequestMock } from '../__mocks__/request.mock';
+import { handleEsError } from '../../shared_imports';
+import { errors as esErrors } from '@elastic/elasticsearch';
 
 const mockReindexService = {
   hasRequiredPrivileges: jest.fn(),
@@ -60,6 +62,7 @@ describe('reindex API', () => {
       credentialStore,
       router: mockRouter,
       licensing: licensingMock.createSetup(),
+      lib: { handleEsError },
       getSecurityPlugin: () => securityMock.createStart(),
     };
     registerReindexIndicesRoutes(routeDependencies, () => worker);
@@ -125,6 +128,24 @@ describe('reindex API', () => {
       ]);
     });
 
+    it('returns es errors', async () => {
+      mockReindexService.findReindexOperation.mockResolvedValueOnce(null);
+      mockReindexService.detectReindexWarnings.mockRejectedValueOnce(
+        new esErrors.ResponseError({ statusCode: 404 } as any)
+      );
+
+      const resp = await routeDependencies.router.getHandler({
+        method: 'get',
+        pathPattern: '/api/upgrade_assistant/reindex/{indexName}',
+      })(
+        routeHandlerContextMock,
+        createRequestMock({ params: { indexName: 'anIndex' } }),
+        kibanaResponseFactory
+      );
+
+      expect(resp.status).toEqual(404);
+    });
+
     it("returns null for both if reindex operation doesn't exist and index doesn't exist", async () => {
       mockReindexService.findReindexOperation.mockResolvedValueOnce(null);
       mockReindexService.detectReindexWarnings.mockResolvedValueOnce(null);