Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { act } from 'react-dom/test-utils';
import { registerTestBed, TestBed, TestBedConfig } from '@kbn/test/jest';

import { AppWithRouter } from '../../../public/application/app';
import { WithAppDependencies } from '../helpers';

const testBedConfig: TestBedConfig = {
memoryRouter: {
initialEntries: [`/overview`],
componentRoutePath: '/overview',
},
doMountAsync: true,
};

export type AppTestBed = TestBed & {
actions: ReturnType<typeof createActions>;
};

const createActions = (testBed: TestBed) => {
const clickDeprecationToggle = async () => {
const { find, component } = testBed;

await act(async () => {
find('deprecationLoggingToggle').simulate('click');
});

component.update();
};

return {
clickDeprecationToggle,
};
};

export const setupAppPage = async (overrides?: Record<string, unknown>): Promise<AppTestBed> => {
const initTestBed = registerTestBed(WithAppDependencies(AppWithRouter, overrides), testBedConfig);
const testBed = await initTestBed();

return {
...testBed,
actions: createActions(testBed),
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { act } from 'react-dom/test-utils';

import { setupEnvironment } from '../helpers';
import { AppTestBed, setupAppPage } from './app.helpers';

describe('Cluster upgrade', () => {
let testBed: AppTestBed;
let server: ReturnType<typeof setupEnvironment>['server'];
let httpRequestsMockHelpers: ReturnType<typeof setupEnvironment>['httpRequestsMockHelpers'];

beforeEach(() => {
({ server, httpRequestsMockHelpers } = setupEnvironment());
});

afterEach(() => {
server.restore();
});

describe('when user is still preparing for upgrade', () => {
beforeEach(async () => {
testBed = await setupAppPage();
});

test('renders overview', () => {
const { exists } = testBed;
expect(exists('overview')).toBe(true);
expect(exists('isUpgradingMessage')).toBe(false);
expect(exists('isUpgradeCompleteMessage')).toBe(false);
});
});

describe('when cluster is in the process of a rolling upgrade', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadDeprecationLoggingResponse(undefined, {
statusCode: 426,
message: '',
attributes: {
allNodesUpgraded: false,
},
});

await act(async () => {
testBed = await setupAppPage();
});
});

test('renders rolling upgrade message', async () => {
const { component, exists } = testBed;
component.update();
expect(exists('overview')).toBe(false);
expect(exists('isUpgradingMessage')).toBe(true);
expect(exists('isUpgradeCompleteMessage')).toBe(false);
});
});

describe('when cluster has been upgraded', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadDeprecationLoggingResponse(undefined, {
statusCode: 426,
message: '',
attributes: {
allNodesUpgraded: true,
},
});

await act(async () => {
testBed = await setupAppPage();
});
});

test('renders upgrade complete message', () => {
const { component, exists } = testBed;
component.update();
expect(exists('overview')).toBe(false);
expect(exists('isUpgradingMessage')).toBe(false);
expect(exists('isUpgradeCompleteMessage')).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,7 @@ export const getAppContextMock = () => ({
isCloudEnabled: false,
},
},
clusterUpgradeState: 'isPreparingForUpgrade',
isClusterUpgradeStateError: () => {},
handleClusterUpgradeStateError: () => {},
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
*/

import sinon, { SinonFakeServer } from 'sinon';

import { API_BASE_PATH } from '../../../common/constants';
import {
CloudBackupStatus,
ESUpgradeStatus,
DeprecationLoggingStatus,
ResponseError,
} from '../../../common/types';
import { ResponseError } from '../../../public/application/lib/api';

// Register helpers to mock HTTP Requests
const registerHttpRequestMockHelpers = (server: SinonFakeServer) => {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/upgrade_assistant/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@ export const DEPRECATION_LOGS_SOURCE_ID = 'deprecation_logs';
export const DEPRECATION_LOGS_INDEX = '.logs-deprecation.elasticsearch-default';
export const DEPRECATION_LOGS_INDEX_PATTERN = '.logs-deprecation.elasticsearch-default';

export const CLUSTER_UPGRADE_STATUS_POLL_INTERVAL_MS = 45000;
export const CLOUD_BACKUP_STATUS_POLL_INTERVAL_MS = 60000;
export const DEPRECATION_LOGS_COUNT_POLL_INTERVAL_MS = 15000;
8 changes: 8 additions & 0 deletions x-pack/plugins/upgrade_assistant/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ import { SavedObject, SavedObjectAttributes } from 'src/core/public';

export type DeprecationSource = 'Kibana' | 'Elasticsearch';

export type ClusterUpgradeState = 'isPreparingForUpgrade' | 'isUpgrading' | 'isUpgradeComplete';

export interface ResponseError {
statusCode: number;
message: string | Error;
attributes?: Record<string, any>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add types for attributes also? It seems it only holds allNodesUpgraded: boolean

}

export enum ReindexStep {
// Enum values are spaced out by 10 to give us room to insert steps in between.
created = 0,
Expand Down
102 changes: 98 additions & 4 deletions x-pack/plugins/upgrade_assistant/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,109 @@
* 2.0.
*/

import React from 'react';
import React, { useState, useEffect } from 'react';
import { Router, Switch, Route, Redirect } from 'react-router-dom';
import { FormattedMessage } from '@kbn/i18n/react';
import { EuiEmptyPrompt, EuiPageContent } from '@elastic/eui';
import { ScopedHistory } from 'src/core/public';

import { RedirectAppLinks } from '../../../../../src/plugins/kibana_react/public';
import { API_BASE_PATH } from '../../common/constants';
import { ClusterUpgradeState } from '../../common/types';
import { APP_WRAPPER_CLASS, GlobalFlyout, AuthorizationProvider } from '../shared_imports';
import { AppDependencies } from '../types';
import { API_BASE_PATH } from '../../common/constants';
import { AppContextProvider, useAppContext } from './app_context';
import { EsDeprecations, ComingSoonPrompt, KibanaDeprecations, Overview } from './components';

const { GlobalFlyoutProvider } = GlobalFlyout;

const App: React.FunctionComponent = () => {
const { isReadOnlyMode } = useAppContext();
const AppWithoutPolling: React.FunctionComponent = () => {
const {
isReadOnlyMode,
services: { api },
} = useAppContext();

const [clusterUpgradeState, setClusterUpradeState] =
useState<ClusterUpgradeState>('isPreparingForUpgrade');

useEffect(() => {
api.onClusterUpgradeStateChange((newClusterUpgradeState: ClusterUpgradeState) => {
setClusterUpradeState(newClusterUpgradeState);
});
}, [api]);

// Read-only mode will be enabled up until the last minor before the next major release
if (isReadOnlyMode) {
return <ComingSoonPrompt />;
}

if (clusterUpgradeState === 'isUpgrading') {
return (
<EuiPageContent
hasShadow={false}
paddingSize="none"
verticalPosition="center"
horizontalPosition="center"
data-test-subj="isUpgradingMessage"
>
<EuiEmptyPrompt
iconType="logoElasticsearch"
title={
<h1>
<FormattedMessage
id="xpack.upgradeAssistant.upgradingTitle"
defaultMessage="Your cluster is upgrading"
/>
</h1>
}
body={
<p>
<FormattedMessage
id="xpack.upgradeAssistant.upgradingDescription"
defaultMessage="One or more Elasticsearch nodes have a newer version of
Elasticsearch than Kibana. Once all your nodes are upgraded, upgrade Kibana."
/>
</p>
}
data-test-subj="emptyPrompt"
/>
</EuiPageContent>
);
}

if (clusterUpgradeState === 'isUpgradeComplete') {
return (
<EuiPageContent
hasShadow={false}
paddingSize="none"
verticalPosition="center"
horizontalPosition="center"
data-test-subj="isUpgradeCompleteMessage"
>
<EuiEmptyPrompt
iconType="logoElasticsearch"
title={
<h1>
<FormattedMessage
id="xpack.upgradeAssistant.upgradedTitle"
defaultMessage="Your cluster has been upgraded"
/>
</h1>
}
body={
<p>
<FormattedMessage
id="xpack.upgradeAssistant.upgradedDescription"
defaultMessage="All Elasticsearch nodes have been upgraded. You may now upgrade Kibana."
/>
</p>
}
data-test-subj="emptyPrompt"
/>
</EuiPageContent>
);
}

return (
<Switch>
<Route exact path="/overview" component={Overview} />
Expand All @@ -36,6 +118,18 @@ const App: React.FunctionComponent = () => {
);
};

export const App: React.FunctionComponent = () => {
const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about merging this component and AppWithRouter? they both feel quite small and it might reduce some of the overhead of reading this file which already has a bunch of components

services: { api },
} = useAppContext();

// This is a hack to avoid the app getting stuck in an infinite render loop,
// as noted in api.ts.
api.useLoadClusterUpgradeStatus();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we check when this is loading and if so show a Loading component on screen? Otherwise theres an awkward flash until this state finished being fetched:

Screen.Recording.2021-10-12.at.15.05.20.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Fixed:
loading_ua

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that there's already a few API's that have polling (logs count, system indices migration and cloud backup status) which could potentially return the 426 error we need to show the interstitials, and also calling any API's when the upgrade process started will also result in a 426 error. What do you think about polling for this one only when the interstitials are visible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea. I'm not sure I see the problem though. I agree that this is designed to be way more defensive than it needs to be (multiple polls and opportunities to detect the 426), but I don't see any downside except that more requests are being sent than are necessary. In terms of practical cost, I don't think the number of requests is high enough to incur a noticeable performance cost. Was there another issue that you spotted?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah not really an issue, was mainly a thought about preventing extra api calls to the server


return <AppWithoutPolling />;
};

export const AppWithRouter = ({ history }: { history: ScopedHistory }) => {
return (
<Router history={history}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ import {
EuiSpacer,
EuiCallOut,
} from '@elastic/eui';
import { EnrichedDeprecationInfo, IndexSettingAction } from '../../../../../../common/types';
import type { ResponseError } from '../../../../lib/api';

import {
EnrichedDeprecationInfo,
IndexSettingAction,
ResponseError,
} from '../../../../../../common/types';
import type { Status } from '../../../types';
import { DeprecationBadge } from '../../../shared';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@

import React, { useState, useEffect, useCallback } from 'react';
import { EuiTableRowCell } from '@elastic/eui';
import { EnrichedDeprecationInfo } from '../../../../../../common/types';
import { EnrichedDeprecationInfo, ResponseError } from '../../../../../../common/types';
import { GlobalFlyout } from '../../../../../shared_imports';
import { useAppContext } from '../../../../app_context';
import type { ResponseError } from '../../../../lib/api';
import { EsDeprecationsTableCells } from '../../es_deprecations_table_cells';
import { DeprecationTableColumns, Status } from '../../../types';
import { IndexSettingsResolutionCell } from './resolution_table_cell';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

import { useRef, useCallback, useState, useEffect } from 'react';

import { ApiService, ResponseError } from '../../../../lib/api';
import { ResponseError } from '../../../../../../common/types';
import { ApiService } from '../../../../lib/api';
import { Status } from '../../../types';

const POLL_INTERVAL_MS = 1000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { ResponseError } from '../../../../lib/api';
import { ResponseError } from '../../../../../../common/types';
import { DeprecationLoggingPreviewProps } from '../../../types';

import './_deprecation_logging_toggle.scss';
Expand Down Expand Up @@ -79,7 +79,18 @@ const ErrorDetailsLink = ({ error }: { error: ResponseError }) => {
);
};

export const DeprecationLoggingToggle: FunctionComponent<DeprecationLoggingPreviewProps> = ({
type Props = Pick<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL !

DeprecationLoggingPreviewProps,
| 'isDeprecationLogIndexingEnabled'
| 'isLoading'
| 'isUpdating'
| 'fetchError'
| 'updateError'
| 'resendRequest'
| 'toggleLogging'
>;

export const DeprecationLoggingToggle: FunctionComponent<Props> = ({
isDeprecationLogIndexingEnabled,
isLoading,
isUpdating,
Expand Down
Loading