Skip to content

Commit

Permalink
[Upgrade Assistant] Better handling of closed indices (#58890)
Browse files Browse the repository at this point in the history
* Exclude disallowed, private setting at index creation

* Remove intl from tabs component

* Added logic for checking the current index status

* Added ES contract integration test

Using _cluster/state is considered internal. This adds an integration
test for checking the contract in CI.

* Add the client side notification for closed indices

* First version of end-to-end functionality working

* Clean up unused, incorrect type information

* Fix type issues and added a comment about the new reindex options

* Fixed server side tests, added comments and logic updates

Updated the handling of reindexOptions to make it more
backwards compatible (treat it as if it could be undefined).

Also update the logic for checking for open or closed indices.
No optional chaining! It should break if the response does not
exactly match.

* Clean up unused code

* Improved idempotency of test and check explicitly for "close".

Rather check for the specific value we want, as this is what is
also gauranteed by the tests. In this way, the information we
send back to the client is also more accurate regarding the index
status. If, in future, more index states are introduced this will
need to be revisited if it affects the ability for an index to be
re-indexed.

* Update client-side tests

* Fix types

* Handle a case where the index name provided may be an alias

* Fix types

* merge-conflict: finish merge conflict resolution

* Update x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/deprecations/reindex/closed_warning_icon.tsx

Co-Authored-By: Alison Goryachev <alisonmllr20@gmail.com>

* merge-conflict: Remove duplicate import

VSCode does not auto-save as expected :sigh:

* ui: Revisit the UI

Moved the warning icon to inside of the button and tooltip to
on the button.

Added a callout to the reindex flyout for when an index is closed.

* logic: slight update to when the index closed callout is shown

We only show the index closed callout in the flyout when the
reindex operation is not considered "completed"

* tests: fix jest tests

* refactor: remove "openAndClose" from reindex endpoints

"openAndClose" should just happen automatically. The user should
not have to pass the flag in, that would be a weird API. We just
need to warn the user about that reindexing a closed index will
take more resources

* test: update upgrade assistant integration test

* fix: types

* copy: use sentence case

* refactor: use the in scope declaration of reindex op

* test: Clean up tests

Reindexing test was generating index name, could just get it from
server response. Also removed openAndClose from all integration
tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
  • Loading branch information
3 people committed Mar 9, 2020
1 parent a6489aa commit 492a97e
Show file tree
Hide file tree
Showing 32 changed files with 596 additions and 126 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { getIndexStateFromClusterState } from './get_index_state_from_cluster_state';
import { ClusterStateAPIResponse } from './types';

describe('getIndexStateFromClusterState', () => {
const indexName = 'indexName';
const clusterState: ClusterStateAPIResponse = {
metadata: {
indices: {},
cluster_coordination: {} as any,
cluster_uuid: 'test',
templates: {} as any,
},
cluster_name: 'test',
cluster_uuid: 'test',
};

afterEach(() => {
clusterState.metadata.indices = {};
});

it('correctly extracts state from cluster state', () => {
clusterState.metadata.indices[indexName] = { state: 'open' } as any;
clusterState.metadata.indices.aTotallyDifferentIndex = { state: 'close' } as any;
expect(getIndexStateFromClusterState(indexName, clusterState)).toBe('open');
});

it('correctly extracts state from aliased index in cluster state', () => {
clusterState.metadata.indices.aTotallyDifferentName = {
state: 'close',
aliases: [indexName, 'test'],
} as any;
clusterState.metadata.indices.aTotallyDifferentName1 = {
state: 'open',
aliases: ['another', 'test'],
} as any;

expect(getIndexStateFromClusterState(indexName, clusterState)).toBe('close');
});

it('throws if the index name cannot be found in the cluster state', () => {
expect(() => getIndexStateFromClusterState(indexName, clusterState)).toThrow('not found');
clusterState.metadata.indices.aTotallyDifferentName1 = {
state: 'open',
aliases: ['another', 'test'],
} as any;
expect(() => getIndexStateFromClusterState(indexName, clusterState)).toThrow('not found');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { ClusterStateAPIResponse } from './types';

const checkAllAliases = (
indexName: string,
clusterState: ClusterStateAPIResponse
): 'open' | 'close' => {
for (const index of Object.values(clusterState.metadata.indices)) {
if (index.aliases?.some(alias => alias === indexName)) {
return index.state;
}
}

throw new Error(`${indexName} not found in cluster state!`);
};

export const getIndexStateFromClusterState = (
indexName: string,
clusterState: ClusterStateAPIResponse
): 'open' | 'close' =>
clusterState.metadata.indices[indexName]
? clusterState.metadata.indices[indexName].state
: checkAllAliases(indexName, clusterState);
55 changes: 54 additions & 1 deletion x-pack/plugins/upgrade_assistant/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export interface QueueSettings extends SavedObjectAttributes {
}

export interface ReindexOptions extends SavedObjectAttributes {
/**
* Whether to treat the index as if it were closed. This instructs the
* reindex strategy to first open the index, perform reindexing and
* then close the index again.
*/
openAndClose?: boolean;

/**
* Set this key to configure a reindex operation as part of a
* batch to be run in series.
Expand All @@ -50,7 +57,6 @@ export interface ReindexOperation extends SavedObjectAttributes {
reindexTaskId: string | null;
reindexTaskPercComplete: number | null;
errorMessage: string | null;

// This field is only used for the singleton IndexConsumerType documents.
runningReindexCount: number | null;

Expand Down Expand Up @@ -142,10 +148,57 @@ export interface EnrichedDeprecationInfo extends DeprecationInfo {
index?: string;
node?: string;
reindex?: boolean;
/**
* Indicate what blockers have been detected for calling reindex
* against this index.
*
* @remark
* In future this could be an array of blockers.
*/
blockerForReindexing?: 'index-closed'; // 'index-closed' can be handled automatically, but requires more resources, user should be warned
}

export interface UpgradeAssistantStatus {
readyForUpgrade: boolean;
cluster: EnrichedDeprecationInfo[];
indices: EnrichedDeprecationInfo[];
}

export interface ClusterStateIndexAPIResponse {
state: 'open' | 'close';
settings: {
index: {
verified_before_close: string;
search: {
throttled: string;
};
number_of_shards: string;
provided_name: string;
frozen: string;
creation_date: string;
number_of_replicas: string;
uuid: string;
version: {
created: string;
};
};
};
mappings: any;
aliases: string[];
}

export interface ClusterStateAPIResponse {
cluster_name: string;
cluster_uuid: string;
metadata: {
cluster_uuid: string;
cluster_coordination: {
term: number;
last_committed_config: string[];
last_accepted_config: string[];
voting_config_exclusions: [];
};
templates: any;
indices: { [indexName: string]: ClusterStateIndexAPIResponse };
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { HttpSetup } from 'src/core/public';
import { DocLinksStart, HttpSetup } from 'src/core/public';
import React, { createContext, useContext } from 'react';

export interface ContextValue {
http: HttpSetup;
isCloudEnabled: boolean;
docLinks: DocLinksStart;
}

export const AppContext = createContext<ContextValue>({} as any);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
EuiTabbedContent,
EuiTabbedContentTab,
} from '@elastic/eui';
import { FormattedMessage, injectI18n } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { HttpSetup } from 'src/core/public';

import { UpgradeAssistantStatus } from '../../../common/types';
Expand All @@ -38,9 +39,11 @@ interface TabsState {
clusterUpgradeState: ClusterUpgradeState;
}

type Props = ReactIntl.InjectedIntlProps & { http: HttpSetup };
interface Props {
http: HttpSetup;
}

export class UpgradeAssistantTabsUI extends React.Component<Props, TabsState> {
export class UpgradeAssistantTabs extends React.Component<Props, TabsState> {
constructor(props: Props) {
super(props);
this.state = {
Expand Down Expand Up @@ -172,7 +175,6 @@ export class UpgradeAssistantTabsUI extends React.Component<Props, TabsState> {
};

private get tabs() {
const { intl } = this.props;
const { loadingError, loadingState, checkupData } = this.state;
const commonProps: UpgradeAssistantTabProps = {
loadingError,
Expand All @@ -186,24 +188,21 @@ export class UpgradeAssistantTabsUI extends React.Component<Props, TabsState> {
return [
{
id: 'overview',
name: intl.formatMessage({
id: 'xpack.upgradeAssistant.overviewTab.overviewTabTitle',
name: i18n.translate('xpack.upgradeAssistant.overviewTab.overviewTabTitle', {
defaultMessage: 'Overview',
}),
content: <OverviewTab checkupData={checkupData} {...commonProps} />,
},
{
id: 'cluster',
name: intl.formatMessage({
id: 'xpack.upgradeAssistant.checkupTab.clusterTabLabel',
name: i18n.translate('xpack.upgradeAssistant.checkupTab.clusterTabLabel', {
defaultMessage: 'Cluster',
}),
content: (
<CheckupTab
key="cluster"
deprecations={checkupData ? checkupData.cluster : undefined}
checkupLabel={intl.formatMessage({
id: 'xpack.upgradeAssistant.tabs.checkupTab.clusterLabel',
checkupLabel={i18n.translate('xpack.upgradeAssistant.tabs.checkupTab.clusterLabel', {
defaultMessage: 'cluster',
})}
{...commonProps}
Expand All @@ -212,16 +211,14 @@ export class UpgradeAssistantTabsUI extends React.Component<Props, TabsState> {
},
{
id: 'indices',
name: intl.formatMessage({
id: 'xpack.upgradeAssistant.checkupTab.indicesTabLabel',
name: i18n.translate('xpack.upgradeAssistant.checkupTab.indicesTabLabel', {
defaultMessage: 'Indices',
}),
content: (
<CheckupTab
key="indices"
deprecations={checkupData ? checkupData.indices : undefined}
checkupLabel={intl.formatMessage({
id: 'xpack.upgradeAssistant.checkupTab.indexLabel',
checkupLabel={i18n.translate('xpack.upgradeAssistant.checkupTab.indexLabel', {
defaultMessage: 'index',
})}
showBackupWarning
Expand Down Expand Up @@ -249,5 +246,3 @@ export class UpgradeAssistantTabsUI extends React.Component<Props, TabsState> {
this.setState({ telemetryState: TelemetryState.Complete });
}
}

export const UpgradeAssistantTabs = injectI18n(UpgradeAssistantTabsUI);
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { FormattedMessage } from '@kbn/i18n/react';
import { ReindexButton } from './reindex';
import { AppContext } from '../../../../app_context';
import { EnrichedDeprecationInfo } from '../../../../../../common/types';

interface DeprecationCellProps {
items?: Array<{ title?: string; body: string }>;
Expand All @@ -26,6 +27,7 @@ interface DeprecationCellProps {
headline?: string;
healthColor?: string;
children?: ReactNode;
reindexBlocker?: EnrichedDeprecationInfo['blockerForReindexing'];
}

/**
Expand All @@ -38,6 +40,7 @@ export const DeprecationCell: FunctionComponent<DeprecationCellProps> = ({
docUrl,
items = [],
children,
reindexBlocker,
}) => (
<div className="upgDeprecationCell">
<EuiFlexGroup responsive={false} wrap alignItems="baseline">
Expand Down Expand Up @@ -79,7 +82,14 @@ export const DeprecationCell: FunctionComponent<DeprecationCellProps> = ({
{reindexIndexName && (
<EuiFlexItem grow={false}>
<AppContext.Consumer>
{({ http }) => <ReindexButton indexName={reindexIndexName} http={http} />}
{({ http, docLinks }) => (
<ReindexButton
docLinks={docLinks}
reindexBlocker={reindexBlocker}
indexName={reindexIndexName}
http={http}
/>
)}
</AppContext.Consumer>
</EuiFlexItem>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import { EuiBasicTable } from '@elastic/eui';
import { injectI18n } from '@kbn/i18n/react';
import { ReindexButton } from './reindex';
import { AppContext } from '../../../../app_context';
import { EnrichedDeprecationInfo } from '../../../../../../common/types';

const PAGE_SIZES = [10, 25, 50, 100, 250, 500, 1000];

export interface IndexDeprecationDetails {
index: string;
reindex: boolean;
blockerForReindexing?: EnrichedDeprecationInfo['blockerForReindexing'];
details?: string;
}

Expand Down Expand Up @@ -68,9 +70,10 @@ export class IndexDeprecationTableUI extends React.Component<
},
];

if (this.actionsColumn) {
// @ts-ignore
columns.push(this.actionsColumn);
const actionsColumn = this.generateActionsColumn();

if (actionsColumn) {
columns.push(actionsColumn as any);
}

const sorting = {
Expand Down Expand Up @@ -134,7 +137,7 @@ export class IndexDeprecationTableUI extends React.Component<
return { totalItemCount, pageSizeOptions, hidePerPageOptions: false };
}

private get actionsColumn() {
private generateActionsColumn() {
// NOTE: this naive implementation assumes all indices in the table are
// should show the reindex button. This should work for known usecases.
const { indices } = this.props;
Expand All @@ -148,7 +151,16 @@ export class IndexDeprecationTableUI extends React.Component<
render(indexDep: IndexDeprecationDetails) {
return (
<AppContext.Consumer>
{({ http }) => <ReindexButton indexName={indexDep.index!} http={http} />}
{({ http, docLinks }) => {
return (
<ReindexButton
docLinks={docLinks}
reindexBlocker={indexDep.blockerForReindexing}
indexName={indexDep.index!}
http={http}
/>
);
}}
</AppContext.Consumer>
);
},
Expand Down
Loading

0 comments on commit 492a97e

Please sign in to comment.