Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Upgrade Assistant] Swapped reindexing flyouts order #115046

Merged
merged 13 commits into from
Oct 19, 2021
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
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -25584,8 +25584,6 @@
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.continueButtonLabel": "再インデックスを続ける",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.insufficientPrivilegeCallout.calloutTitle": "このインデックスを再インデックスするための権限がありません",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.backgroundResumeDetail": "再インデックスはバックグラウンドで継続しますが、Kibana がシャットダウンまたは再起動した場合、このページに戻り再インデックスを再開させる必要があります。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.calloutTitle": "インデックスは再インデックス中にドキュメントを投入、更新、または削除できません",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.cantStopDetail": "ドキュメントの更新を停止できない場合、または新しいクラスターに再インデックスする必要がある場合は、異なるアップグレード方法をお勧めします。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.reindexingLabel": "再インデックス中…",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.resumeLabel": "再開",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.runReindexLabel": "再インデックスを実行",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -26017,8 +26017,6 @@
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.continueButtonLabel": "继续重新索引",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.insufficientPrivilegeCallout.calloutTitle": "您没有足够的权限来重新索引此索引",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.backgroundResumeDetail": "重新索引将在后台继续,但如果 Kibana 关闭或重新启动,您将需要返回此页,才能恢复重新索引。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.calloutTitle": "在重新索引时,索引无法采集、更新或删除文档",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.cantStopDetail": "如果您无法停止文档更新或需要重新索引到新的集群中,请考虑使用不同的升级策略。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.reindexingLabel": "正在重新索引……",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.resumeLabel": "恢复",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.runReindexLabel": "运行重新索引",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
EuiFlyoutBody,
EuiFlyoutFooter,
EuiSpacer,
EuiText,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

Expand Down Expand Up @@ -122,31 +123,21 @@ export const ChecklistFlyoutStep: React.FunctionComponent<{
</EuiCallOut>
</>
)}
<EuiCallOut
title={
<FormattedMessage
id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.calloutTitle"
defaultMessage="Index is unable to ingest, update, or delete documents while reindexing"
/>
}
color="warning"
iconType="alert"
>
<EuiText>
<p>
<FormattedMessage
id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.cantStopDetail"
defaultMessage="If you can’t stop document updates or need to reindex into a new cluster,
consider using a different upgrade strategy."
id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexDescription"
defaultMessage="Index is unable to ingest, update, or delete documents while reindexing. If you can’t stop document updates or need to reindex into a new cluster, consider using a different upgrade strategy."
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not just "consider using a different upgrade strategy", it's more "don't reindex this index through the upgrade assistant". And the "different upgrade strategy" is really wait until you can stop writing to the index. Which is probably too much to try to convey here.

Suggested change
defaultMessage="Index is unable to ingest, update, or delete documents while reindexing. If you can’t stop document updates or need to reindex into a new cluster, consider using a different upgrade strategy."
defaultMessage="The index will be read-only during reindexing. You won't be able to add, update, or delete documents until reindexing is complete. If you need to reindex to a new cluster, use the reindex API. Learn more"

The 7.16 docs don't exist yet, so this should use a short link we can change. In the meantime, https://www.elastic.co/guide/en/elasticsearch/reference/7.15/docs-reindex.html#reindex-from-remote

/>
</p>
<p>
<FormattedMessage
id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.backgroundResumeDetail"
defaultMessage="Reindexing will continue in the background, but if Kibana shuts down or restarts you will
need to return to this page to resume reindexing."
defaultMessage="You can close this page while reindexing is in progress, it will continue in the background. If Kibana shuts down or restarts, return
to this page to resume reindexing."
Copy link
Contributor

Choose a reason for hiding this comment

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

Kibana doesn't run the reindex request asynchronously? Page doesn't seem quite right for the flyout.

Suggested change
defaultMessage="You can close this page while reindexing is in progress, it will continue in the background. If Kibana shuts down or restarts, return
to this page to resume reindexing."
defaultMessage="Reindexing is performed in the background. You can return to the Upgrade Assistant to view progress or resume reindexing after a Kibana restart."

/>
</p>
</EuiCallOut>
</EuiText>
<EuiSpacer />
<ReindexProgress reindexState={reindexState} cancelReindex={cancelReindex} />
</EuiFlyoutBody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ import { ChecklistFlyoutStep } from './checklist_step';
import { WarningsFlyoutStep } from './warnings_step';
import { DeprecationBadge } from '../../../../shared';

enum ReindexFlyoutStep {
reindexWarnings,
checklist,
}

export interface ReindexFlyoutProps extends ReindexStateContext {
deprecation: EnrichedDeprecationInfo;
closeFlyout: () => void;
Expand All @@ -36,45 +31,44 @@ export const ReindexFlyout: React.FunctionComponent<ReindexFlyoutProps> = ({
const { status, reindexWarnings } = reindexState;
const { index } = deprecation;

// If there are any warnings and we haven't started reindexing, show the warnings step first.
const [currentFlyoutStep, setCurrentFlyoutStep] = useState<ReindexFlyoutStep>(
reindexWarnings && reindexWarnings.length > 0 && status === undefined
? ReindexFlyoutStep.reindexWarnings
: ReindexFlyoutStep.checklist
);

let flyoutContents: React.ReactNode;
const [showWarningsStep, setShowWarningsStep] = useState(false);

switch (currentFlyoutStep) {
case ReindexFlyoutStep.reindexWarnings:
flyoutContents = (
<WarningsFlyoutStep
closeFlyout={closeFlyout}
warnings={reindexState.reindexWarnings!}
advanceNextStep={() => setCurrentFlyoutStep(ReindexFlyoutStep.checklist)}
/>
);
break;
case ReindexFlyoutStep.checklist:
flyoutContents = (
<ChecklistFlyoutStep
closeFlyout={closeFlyout}
reindexState={reindexState}
startReindex={startReindex}
cancelReindex={cancelReindex}
/>
);
break;
default:
throw new Error(`Invalid flyout step: ${currentFlyoutStep}`);
}
const startReindexWithWarnings = () => {
if (
reindexWarnings &&
reindexWarnings.length > 0 &&
status !== ReindexStatus.inProgress &&
status !== ReindexStatus.completed
) {
setShowWarningsStep(true);
} else {
startReindex();
}
};
const flyoutContents = showWarningsStep ? (
<WarningsFlyoutStep
warnings={reindexState.reindexWarnings ?? []}
hideWarningsStep={() => setShowWarningsStep(false)}
continueReindex={() => {
setShowWarningsStep(false);
startReindex();
}}
/>
) : (
<ChecklistFlyoutStep
closeFlyout={closeFlyout}
startReindex={startReindexWithWarnings}
reindexState={reindexState}
cancelReindex={cancelReindex}
/>
);

return (
<>
<EuiFlyoutHeader hasBorder>
<DeprecationBadge
isCritical={deprecation.isCritical}
isResolved={reindexState.status === ReindexStatus.completed}
isResolved={status === ReindexStatus.completed}
/>
<EuiSpacer size="s" />
<EuiTitle size="s" data-test-subj="flyoutTitle">
Expand All @@ -87,6 +81,7 @@ export const ReindexFlyout: React.FunctionComponent<ReindexFlyoutProps> = ({
</h2>
</EuiTitle>
</EuiFlyoutHeader>

{flyoutContents}
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ jest.mock('../../../../../app_context', () => {

describe('WarningsFlyoutStep', () => {
const defaultProps = {
advanceNextStep: jest.fn(),
warnings: [] as ReindexWarning[],
closeFlyout: jest.fn(),
renderGlobalCallouts: jest.fn(),
hideWarningsStep: jest.fn(),
continueReindex: jest.fn(),
};

it('renders', () => {
Expand Down Expand Up @@ -71,15 +70,15 @@ describe('WarningsFlyoutStep', () => {
const button = wrapper.find('EuiButton');

button.simulate('click');
expect(defaultPropsWithWarnings.advanceNextStep).not.toHaveBeenCalled();
expect(defaultPropsWithWarnings.continueReindex).not.toHaveBeenCalled();

// first warning (customTypeName)
wrapper.find(`input#${idForWarning(0)}`).simulate('change');
// second warning (indexSetting)
wrapper.find(`input#${idForWarning(1)}`).simulate('change');
button.simulate('click');

expect(defaultPropsWithWarnings.advanceNextStep).toHaveBeenCalled();
expect(defaultPropsWithWarnings.continueReindex).toHaveBeenCalled();
});
}
});
Loading