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

[ILM ] Fix logic for showing/hiding recommended allocation on Cloud #90592

Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a197a5a
updated logic for hiding recommended allocation options on cloud and …
jloleysens Feb 8, 2021
40a1858
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 8, 2021
7ed5223
added version check and tests for version check to enable pre v8 beha…
jloleysens Feb 8, 2021
dc70ff9
implement feedback to make tests more legible, fix test names and min…
jloleysens Feb 9, 2021
e11c708
added additional callout for data tier state, also added some new cop…
jloleysens Feb 9, 2021
37ddcfd
remove unused stackVersion context value
jloleysens Feb 9, 2021
39ba98f
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 9, 2021
48099a8
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 9, 2021
b523a5f
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 11, 2021
d9529f5
address windows max path length constraint
jloleysens Feb 11, 2021
6d933b8
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 15, 2021
28ed48d
Merge branch 'master' of github.com:elastic/kibana into ilm/update-co…
jloleysens Feb 15, 2021
9b632d7
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 17, 2021
b4d6ef5
Merge branch 'master' of github.com:elastic/kibana into ilm/update-co…
jloleysens Feb 22, 2021
b6bfe49
- Fix botched conflict resolution!
jloleysens Feb 22, 2021
7746647
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 23, 2021
437962b
Merge branch 'master' into ilm/update-condition-for-hiding-recommded-…
kibanamachine Feb 25, 2021
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
Expand Up @@ -174,11 +174,15 @@ export const setup = async (arg?: { appServicesContext: Partial<AppServicesConte
const setMinAgeUnits = (phase: Phases) =>
createFormSetValueAction(`${phase}-selectedMinimumAgeUnits`);

const setDataAllocation = (phase: Phases) => async (value: DataTierAllocationType) => {
const showDataAllocationOptions = (phase: Phases) => () => {
act(() => {
find(`${phase}-dataTierAllocationControls.dataTierSelect`).simulate('click');
});
component.update();
};

const setDataAllocation = (phase: Phases) => async (value: DataTierAllocationType) => {
showDataAllocationOptions(phase)();
await act(async () => {
switch (value) {
case 'node_roles':
Expand Down Expand Up @@ -305,6 +309,7 @@ export const setup = async (arg?: { appServicesContext: Partial<AppServicesConte
enable: enable('warm'),
setMinAgeValue: setMinAgeValue('warm'),
setMinAgeUnits: setMinAgeUnits('warm'),
showDataAllocationOptions: showDataAllocationOptions('warm'),
setDataAllocation: setDataAllocation('warm'),
setSelectedNodeAttribute: setSelectedNodeAttribute('warm'),
setReplicas: setReplicas('warm'),
Expand All @@ -319,6 +324,7 @@ export const setup = async (arg?: { appServicesContext: Partial<AppServicesConte
enable: enable('cold'),
setMinAgeValue: setMinAgeValue('cold'),
setMinAgeUnits: setMinAgeUnits('cold'),
showDataAllocationOptions: showDataAllocationOptions('cold'),
setDataAllocation: setDataAllocation('cold'),
setSelectedNodeAttribute: setSelectedNodeAttribute('cold'),
setReplicas: setReplicas('cold'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,43 +672,52 @@ describe('<EditPolicy />', () => {
expect(find('cold-dataTierAllocationControls.dataTierSelect').text()).toContain('Off');
});
});
});

describe('searchable snapshot', () => {
describe('on cloud', () => {
describe('new policy', () => {
describe('using legacy data role config', () => {
beforeEach(async () => {
// simulate creating a new policy
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('')]);
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]);
httpRequestsMockHelpers.setListNodes({
isUsingDeprecatedDataRoleConfig: false,
nodesByAttributes: { test: ['123'] },
nodesByRoles: { data: ['123'] },
// On cloud, even if there are data_* roles set, the default, recommended allocation option should not
// be available.
nodesByRoles: { data_hot: ['123'] },
isUsingDeprecatedDataRoleConfig: true,
});
httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] });

await act(async () => {
testBed = await setup({ appServicesContext: { cloud: { isCloudEnabled: true } } });
testBed = await setup({
appServicesContext: {
cloud: {
isCloudEnabled: true,
},
license: licensingMock.createLicense({ license: { type: 'basic' } }),
},
});
});

const { component } = testBed;
component.update();
});
test('defaults searchable snapshot to true on cloud', async () => {
const { find, actions } = testBed;
await actions.cold.enable(true);
expect(
find('searchableSnapshotField-cold.searchableSnapshotToggle').props()['aria-checked']
).toBe(true);
test('removes default, recommended option', async () => {
const { actions, find } = testBed;
await actions.warm.enable(true);
actions.warm.showDataAllocationOptions();

expect(find('defaultDataAllocationOption').exists()).toBeFalsy();
expect(find('customDataAllocationOption').exists()).toBeTruthy();
expect(find('noneDataAllocationOption').exists()).toBeTruthy();
// Show the call-to-action for users to migrate their cluster to use node roles
expect(find('cloudDataTierCallout').exists()).toBeTruthy();
});
});
describe('existing policy', () => {
describe('using node roles', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]);
httpRequestsMockHelpers.setListNodes({
isUsingDeprecatedDataRoleConfig: false,
nodesByAttributes: { test: ['123'] },
nodesByRoles: { data: ['123'] },
nodesByRoles: { data_hot: ['123'] },
isUsingDeprecatedDataRoleConfig: false,
});
httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] });

Expand All @@ -719,19 +728,34 @@ describe('<EditPolicy />', () => {
const { component } = testBed;
component.update();
});
test('correctly sets snapshot repository default to "found-snapshots"', async () => {
const { actions } = testBed;

test('should show off, custom and data role options on cloud with data roles', async () => {
const { actions, find } = testBed;

await actions.warm.enable(true);
actions.warm.showDataAllocationOptions();
expect(find('defaultDataAllocationOption').exists()).toBeTruthy();
expect(find('customDataAllocationOption').exists()).toBeTruthy();
expect(find('noneDataAllocationOption').exists()).toBeTruthy();
// We should not be showing the call-to-action for users to activate the cold tier on cloud
expect(find('cloudMissingColdTierCallout').exists()).toBeFalsy();
// Do not show the call-to-action for users to migrate their cluster to use node roles
expect(find('cloudDataTierCallout').exists()).toBeFalsy();
});

test('should show cloud notice when cold tier nodes do not exist', async () => {
const { actions, find } = testBed;
await actions.cold.enable(true);
await actions.cold.toggleSearchableSnapshot(true);
await actions.savePolicy();
const latestRequest = server.requests[server.requests.length - 1];
const request = JSON.parse(JSON.parse(latestRequest.requestBody).body);
expect(request.phases.cold.actions.searchable_snapshot.snapshot_repository).toEqual(
'found-snapshots'
);
expect(find('cloudMissingColdTierCallout').exists()).toBeTruthy();
// Assert that other notices are not showing
expect(find('defaultAllocationNotice').exists()).toBeFalsy();
expect(find('noNodeAttributesWarning').exists()).toBeFalsy();
});
});
});
});

describe('searchable snapshot', () => {
describe('on non-enterprise license', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]);
Expand Down Expand Up @@ -768,6 +792,64 @@ describe('<EditPolicy />', () => {
expect(actions.cold.searchableSnapshotDisabledDueToLicense()).toBeTruthy();
});
});

describe('on cloud', () => {
describe('new policy', () => {
beforeEach(async () => {
// simulate creating a new policy
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('')]);
httpRequestsMockHelpers.setListNodes({
nodesByAttributes: { test: ['123'] },
nodesByRoles: { data: ['123'] },
isUsingDeprecatedDataRoleConfig: false,
});
httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] });

await act(async () => {
testBed = await setup({ appServicesContext: { cloud: { isCloudEnabled: true } } });
});

const { component } = testBed;
component.update();
});
test('defaults searchable snapshot to true on cloud', async () => {
const { find, actions } = testBed;
await actions.cold.enable(true);
expect(
find('searchableSnapshotField-cold.searchableSnapshotToggle').props()['aria-checked']
).toBe(true);
});
});
describe('existing policy', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]);
httpRequestsMockHelpers.setListNodes({
isUsingDeprecatedDataRoleConfig: false,
nodesByAttributes: { test: ['123'] },
nodesByRoles: { data_hot: ['123'] },
});
httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] });

await act(async () => {
testBed = await setup({ appServicesContext: { cloud: { isCloudEnabled: true } } });
});

const { component } = testBed;
component.update();
});
test('correctly sets snapshot repository default to "found-snapshots"', async () => {
const { actions } = testBed;
await actions.cold.enable(true);
await actions.cold.toggleSearchableSnapshot(true);
await actions.savePolicy();
const latestRequest = server.requests[server.requests.length - 1];
const request = JSON.parse(JSON.parse(latestRequest.requestBody).body);
expect(request.phases.cold.actions.searchable_snapshot.snapshot_repository).toEqual(
'found-snapshots'
);
});
});
});
});
describe('with rollover', () => {
beforeEach(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -884,84 +884,4 @@ describe('edit policy', () => {
expect(findTestSubject(rendered, 'noneDataAllocationOption').exists()).toBeTruthy();
});
});
describe('on cloud', () => {
beforeEach(() => {
component = (
<MyComponent
isNewPolicy={true}
policy={defaultPolicy}
existingPolicies={policies}
getUrlForApp={jest.fn()}
isCloudEnabled={true}
license={{ canUseSearchableSnapshot: () => true }}
/>
);
({ http } = editPolicyHelpers.setup());
({ server, httpRequestsMockHelpers } = http);
server.respondImmediately = true;

httpRequestsMockHelpers.setPoliciesResponse(policies);
});

describe('with deprecated data role config', () => {
test('should hide data tier option on cloud using legacy node role configuration', async () => {
http.setupNodeListResponse({
nodesByAttributes: { test: ['123'] },
// On cloud, if using legacy config there will not be any "data_*" roles set.
nodesByRoles: { data: ['test'] },
isUsingDeprecatedDataRoleConfig: true,
});
const rendered = mountWithIntl(component);
await noRollover(rendered);
await setPolicyName(rendered, 'mypolicy');
await activatePhase(rendered, 'warm');
expect(rendered.find('.euiLoadingSpinner').exists()).toBeFalsy();

// Assert that default, custom and 'none' options exist
findTestSubject(rendered, 'dataTierSelect').simulate('click');
expect(findTestSubject(rendered, 'defaultDataAllocationOption').exists()).toBeFalsy();
expect(findTestSubject(rendered, 'customDataAllocationOption').exists()).toBeTruthy();
expect(findTestSubject(rendered, 'noneDataAllocationOption').exists()).toBeTruthy();
});
});

describe('with node role config', () => {
test('should show off, custom and data role options on cloud with data roles', async () => {
http.setupNodeListResponse({
nodesByAttributes: { test: ['123'] },
nodesByRoles: { data: ['test'], data_hot: ['test'], data_warm: ['test'] },
isUsingDeprecatedDataRoleConfig: false,
});
const rendered = mountWithIntl(component);
await noRollover(rendered);
await setPolicyName(rendered, 'mypolicy');
await activatePhase(rendered, 'warm');
expect(rendered.find('.euiLoadingSpinner').exists()).toBeFalsy();

findTestSubject(rendered, 'dataTierSelect').simulate('click');
expect(findTestSubject(rendered, 'defaultDataAllocationOption').exists()).toBeTruthy();
expect(findTestSubject(rendered, 'customDataAllocationOption').exists()).toBeTruthy();
expect(findTestSubject(rendered, 'noneDataAllocationOption').exists()).toBeTruthy();
// We should not be showing the call-to-action for users to activate data tiers in cloud
expect(findTestSubject(rendered, 'cloudDataTierCallout').exists()).toBeFalsy();
});

test('should show cloud notice when cold tier nodes do not exist', async () => {
http.setupNodeListResponse({
nodesByAttributes: {},
nodesByRoles: { data: ['test'], data_hot: ['test'], data_warm: ['test'] },
isUsingDeprecatedDataRoleConfig: false,
});
const rendered = mountWithIntl(component);
await noRollover(rendered);
await setPolicyName(rendered, 'mypolicy');
await activatePhase(rendered, 'cold');
expect(rendered.find('.euiLoadingSpinner').exists()).toBeFalsy();
expect(findTestSubject(rendered, 'cloudDataTierCallout').exists()).toBeTruthy();
// Assert that other notices are not showing
expect(findTestSubject(rendered, 'defaultAllocationNotice').exists()).toBeFalsy();
expect(findTestSubject(rendered, 'noNodeAttributesWarning').exists()).toBeFalsy();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,39 @@

import { i18n } from '@kbn/i18n';
import React, { FunctionComponent } from 'react';
import { EuiCallOut } from '@elastic/eui';
import { EuiCallOut, EuiLink } from '@elastic/eui';

const i18nTexts = {
title: i18n.translate('xpack.indexLifecycleMgmt.editPolicy.cloudDataTierCallout.title', {
defaultMessage: 'Create a cold tier',
defaultMessage: 'Migrate to data tiers',
}),
body: i18n.translate('xpack.indexLifecycleMgmt.editPolicy.cloudDataTierCallout.body', {
defaultMessage: 'Edit your Elastic Cloud deployment to set up a cold tier.',
defaultMessage: 'Migrate your Elastic Cloud deployment to use data tiers.',
}),
linkText: i18n.translate(
'xpack.indexLifecycleMgmt.editPolicy.cloudDataTierCallout.linkToCloudDeploymentDescription',
{ defaultMessage: 'View cloud deployment' }
),
};

export const CloudDataTierCallout: FunctionComponent = () => {
interface Props {
linkToCloudDeployment?: string;
}

/**
* A call-to-action for users to migrate to data tiers if their cluster is still running
* the deprecated node.data:true config.
*/
export const CloudDataTierCallout: FunctionComponent<Props> = ({ linkToCloudDeployment }) => {
return (
<EuiCallOut title={i18nTexts.title} data-test-subj="cloudDataTierCallout">
{i18nTexts.body}
&nbsp;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a non-breaking space, which can interfere with the way words naturally break at the end of a line and wrap to the next line. In most cases we just want a regular breaking space, which would mean replacing this with {' '}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The use of   here was intentional, but I see your point that it is unnecessary.

{Boolean(linkToCloudDeployment) && (
<EuiLink href={linkToCloudDeployment} external>
{i18nTexts.linkText}
</EuiLink>
)}
</EuiCallOut>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export { DefaultAllocationNotice } from './default_allocation_notice';

export { NoNodeAttributesWarning } from './no_node_attributes_warning';

export { MissingColdTierCallout } from './missing_cold_tier_callout';

export { CloudDataTierCallout } from './cloud_data_tier_callout';

export { LoadingError } from './loading_error';
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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 { i18n } from '@kbn/i18n';
import React, { FunctionComponent } from 'react';
import { EuiCallOut, EuiLink } from '@elastic/eui';

const i18nTexts = {
title: i18n.translate('xpack.indexLifecycleMgmt.editPolicy.cloudMissingColdTierCallout.title', {
defaultMessage: 'Create a cold tier',
}),
body: i18n.translate('xpack.indexLifecycleMgmt.editPolicy.cloudMissingColdTierCallout.body', {
defaultMessage: 'Edit your Elastic Cloud deployment to set up a cold tier.',
}),
linkText: i18n.translate(
'xpack.indexLifecycleMgmt.editPolicy.cloudMissingColdTierCallout.linkToCloudDeploymentDescription',
{ defaultMessage: 'View cloud deployment' }
),
};

interface Props {
linkToCloudDeployment?: string;
}

/**
* A call-to-action for users to activate their cold tier slider to provision cold tier nodes.
* This may need to be change when we have autoscaling enabled on a cluster because nodes may not
* yet exist, but will automatically be provisioned.
*/
export const MissingColdTierCallout: FunctionComponent<Props> = ({ linkToCloudDeployment }) => {
return (
<EuiCallOut title={i18nTexts.title} data-test-subj="cloudMissingColdTierCallout">
{i18nTexts.body}
&nbsp;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

{Boolean(linkToCloudDeployment) && (
<EuiLink href={linkToCloudDeployment} external>
{i18nTexts.linkText}
</EuiLink>
)}
</EuiCallOut>
);
};
Loading