[ML] Fix deletion of models that are not used by pipelines (#114107)

* [ML] Fix deletion of models that are not used by pipelines

* [ML] Edits from review

* [ML] Fix jest test for index switch in delete job modal

* [ML] Fix API test calls to createTestTrainedModels

* [ML] Remove unnecessary async from jest test
This commit is contained in:
Pete Harverson 2021-10-07 21:08:13 +01:00 committed by GitHub
parent e62b752516
commit 0b14195b69
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 376 additions and 26 deletions

View file

@ -69,7 +69,7 @@ describe('DeleteAction', () => {
});
describe('When delete model is open', () => {
it('should allow to delete target index by default.', () => {
it('should not allow to delete target index by default.', () => {
const mock = jest.spyOn(CheckPrivilige, 'checkPermission');
mock.mockImplementation((p) => p === 'canDeleteDataFrameAnalytics');
@ -101,10 +101,9 @@ describe('DeleteAction', () => {
const deleteButton = getByTestId('mlAnalyticsJobDeleteButton');
fireEvent.click(deleteButton);
expect(getByTestId('mlAnalyticsJobDeleteModal')).toBeInTheDocument();
expect(getByTestId('mlAnalyticsJobDeleteIndexSwitch')).toBeInTheDocument();
const mlAnalyticsJobDeleteIndexSwitch = getByTestId('mlAnalyticsJobDeleteIndexSwitch');
expect(mlAnalyticsJobDeleteIndexSwitch).toHaveAttribute('aria-checked', 'true');
expect(queryByTestId('mlAnalyticsJobDeleteIndexSwitch')).toBeNull();
expect(queryByTestId('mlAnalyticsJobDeleteIndexPatternSwitch')).toBeNull();
mock.mockRestore();
});
});

View file

@ -89,9 +89,9 @@ export const useDeleteAction = (canDeleteDataFrameAnalytics: boolean) => {
);
}
};
const checkUserIndexPermission = () => {
const checkUserIndexPermission = async () => {
try {
const userCanDelete = canDeleteIndex(indexName, toastNotificationService);
const userCanDelete = await canDeleteIndex(indexName, toastNotificationService);
if (userCanDelete) {
setUserCanDeleteIndex(true);
}

View file

@ -30,7 +30,11 @@ export const DeleteModelsModal: FC<DeleteModelsModalProps> = ({ models, onClose
.map((model) => model.model_id);
return (
<EuiModal onClose={onClose.bind(null, false)} initialFocus="[name=cancelModelDeletion]">
<EuiModal
onClose={onClose.bind(null, false)}
initialFocus="[name=cancelModelDeletion]"
data-test-subj="mlModelsDeleteModal"
>
<EuiModalHeader>
<EuiModalHeaderTitle>
<FormattedMessage
@ -72,7 +76,12 @@ export const DeleteModelsModal: FC<DeleteModelsModalProps> = ({ models, onClose
/>
</EuiButtonEmpty>
<EuiButton onClick={onClose.bind(null, true)} fill color="danger">
<EuiButton
onClick={onClose.bind(null, true)}
fill
color="danger"
data-test-subj="mlModelsDeleteModalConfirmButton"
>
<FormattedMessage
id="xpack.ml.trainedModels.modelsList.deleteModal.deleteButtonLabel"
defaultMessage="Delete"

View file

@ -50,6 +50,7 @@ import {
import { ML_PAGES } from '../../../../../../../common/constants/locator';
import { DataFrameAnalysisConfigType } from '../../../../../../../common/types/data_frame_analytics';
import { timeFormatter } from '../../../../../../../common/util/date_utils';
import { isPopulatedObject } from '../../../../../../../common';
import { ListingPageUrlState } from '../../../../../../../common/types/common';
import { usePageUrlState } from '../../../../../util/url_state';
import { BUILT_IN_MODEL_TAG } from '../../../../../../../common/constants/data_frame_analytics';
@ -342,6 +343,7 @@ export const ModelsList: FC = () => {
description: i18n.translate('xpack.ml.trainedModels.modelsList.deleteModelActionLabel', {
defaultMessage: 'Delete model',
}),
'data-test-subj': 'mlModelsTableRowDeleteAction',
icon: 'trash',
type: 'icon',
color: 'danger',
@ -353,7 +355,7 @@ export const ModelsList: FC = () => {
enabled: (item) => {
// TODO check for permissions to delete ingest pipelines.
// ATM undefined means pipelines fetch failed server-side.
return !item.pipelines;
return !isPopulatedObject(item.pipelines);
},
},
];
@ -389,6 +391,7 @@ export const ModelsList: FC = () => {
iconType={itemIdToExpandedRowMap[item.model_id] ? 'arrowUp' : 'arrowDown'}
/>
),
'data-test-subj': 'mlModelsTableRowDetailsToggle',
},
{
field: ModelsTableToConfigMapping.id,
@ -397,6 +400,7 @@ export const ModelsList: FC = () => {
}),
sortable: true,
truncateText: true,
'data-test-subj': 'mlModelsTableColumnId',
},
{
field: ModelsTableToConfigMapping.description,
@ -406,6 +410,7 @@ export const ModelsList: FC = () => {
}),
sortable: false,
truncateText: true,
'data-test-subj': 'mlModelsTableColumnDescription',
},
{
field: ModelsTableToConfigMapping.type,
@ -418,11 +423,14 @@ export const ModelsList: FC = () => {
<EuiFlexGroup gutterSize={'xs'} wrap>
{types.map((type) => (
<EuiFlexItem key={type} grow={false}>
<EuiBadge color="hollow">{type}</EuiBadge>
<EuiBadge color="hollow" data-test-subj="mlModelType">
{type}
</EuiBadge>
</EuiFlexItem>
))}
</EuiFlexGroup>
),
'data-test-subj': 'mlModelsTableColumnType',
},
{
field: ModelsTableToConfigMapping.createdAt,
@ -432,12 +440,14 @@ export const ModelsList: FC = () => {
dataType: 'date',
render: timeFormatter,
sortable: true,
'data-test-subj': 'mlModelsTableColumnCreatedAt',
},
{
name: i18n.translate('xpack.ml.trainedModels.modelsList.actionsHeader', {
defaultMessage: 'Actions',
}),
actions,
'data-test-subj': 'mlModelsTableColumnActions',
},
];
@ -492,8 +502,7 @@ export const ModelsList: FC = () => {
defaultMessage: 'Select a model',
});
}
if (Array.isArray(item.pipelines) && item.pipelines.length > 0) {
if (isPopulatedObject(item.pipelines)) {
return i18n.translate('xpack.ml.trainedModels.modelsList.disableSelectableMessage', {
defaultMessage: 'Model has associated pipelines',
});
@ -507,7 +516,7 @@ export const ModelsList: FC = () => {
return '';
},
selectable: (item) => !item.pipelines && !isBuiltInModel(item),
selectable: (item) => !isPopulatedObject(item.pipelines) && !isBuiltInModel(item),
onSelectionChange: (selectedItems) => {
setSelectedModels(selectedItems);
},
@ -574,6 +583,7 @@ export const ModelsList: FC = () => {
pagination={pagination}
onTableChange={onTableChange}
sorting={sorting}
data-test-subj={isLoading ? 'mlModelsTable loading' : 'mlModelsTable loaded'}
/>
</div>
{modelsToDelete.length > 0 && (

View file

@ -17,7 +17,7 @@ export default ({ getService }: FtrProviderContext) => {
describe('DELETE trained_models', () => {
before(async () => {
await ml.testResources.setKibanaTimeZoneToUTC();
await ml.api.createdTestTrainedModels('regression', 2);
await ml.api.createTestTrainedModels('regression', 2);
});
after(async () => {

View file

@ -19,7 +19,7 @@ export default ({ getService }: FtrProviderContext) => {
before(async () => {
await ml.testResources.setKibanaTimeZoneToUTC();
testModelIds = await ml.api.createdTestTrainedModels('regression', 2, true);
testModelIds = await ml.api.createTestTrainedModels('regression', 2, true);
});
after(async () => {

View file

@ -17,7 +17,7 @@ export default ({ getService }: FtrProviderContext) => {
describe('GET trained_models/_stats', () => {
before(async () => {
await ml.testResources.setKibanaTimeZoneToUTC();
await ml.api.createdTestTrainedModels('regression', 2);
await ml.api.createTestTrainedModels('regression', 2);
});
after(async () => {

View file

@ -19,7 +19,7 @@ export default ({ getService }: FtrProviderContext) => {
before(async () => {
await ml.testResources.setKibanaTimeZoneToUTC();
testModelIds = await ml.api.createdTestTrainedModels('regression', 5, true);
testModelIds = await ml.api.createTestTrainedModels('regression', 5, true);
await ml.api.createModelAlias('dfa_regression_model_n_0', 'dfa_regression_model_alias');
await ml.api.createIngestPipeline('dfa_regression_model_alias');
});

View file

@ -12,8 +12,8 @@ export default function ({ getService }: FtrProviderContext) {
describe('trained models', function () {
before(async () => {
await ml.trainedModels.createdTestTrainedModels('classification', 15);
await ml.trainedModels.createdTestTrainedModels('regression', 15);
await ml.trainedModels.createTestTrainedModels('classification', 15, true);
await ml.trainedModels.createTestTrainedModels('regression', 15);
await ml.securityUI.loginAsMlPowerUser();
await ml.navigation.navigateToTrainedModels();
});
@ -22,10 +22,116 @@ export default function ({ getService }: FtrProviderContext) {
await ml.api.cleanMlIndices();
});
// 'Created at' will be different on each run,
// so we will just assert that the value is in the expected timestamp format.
const builtInModelData = {
modelId: 'lang_ident_model_1',
description: 'Model used for identifying language from arbitrary input text.',
modelTypes: ['classification', 'built-in'],
};
const modelWithPipelineData = {
modelId: 'dfa_classification_model_n_0',
description: '',
modelTypes: ['classification'],
};
const modelWithoutPipelineData = {
modelId: 'dfa_regression_model_n_0',
description: '',
modelTypes: ['regression'],
};
it('renders trained models list', async () => {
await ml.trainedModels.assertRowsNumberPerPage(10);
await ml.testExecution.logTestStep(
'should display the stats bar with the total number of models'
);
// +1 because of the built-in model
await ml.trainedModels.assertStats(31);
await ml.testExecution.logTestStep('should display the table');
await ml.trainedModels.assertTableExists();
await ml.trainedModels.assertRowsNumberPerPage(10);
});
it('displays the built-in model and no actions are enabled', async () => {
await ml.testExecution.logTestStep('should display the model in the table');
await ml.trainedModelsTable.filterWithSearchString(builtInModelData.modelId, 1);
await ml.testExecution.logTestStep('displays expected row values for the model in the table');
await ml.trainedModelsTable.assertModelsRowFields(builtInModelData.modelId, {
id: builtInModelData.modelId,
description: builtInModelData.description,
modelTypes: builtInModelData.modelTypes,
});
await ml.testExecution.logTestStep(
'should not show collapsed actions menu for the model in the table'
);
await ml.trainedModelsTable.assertModelCollapsedActionsButtonExists(
builtInModelData.modelId,
false
);
await ml.testExecution.logTestStep(
'should not show delete action for the model in the table'
);
await ml.trainedModelsTable.assertModelDeleteActionButtonExists(
builtInModelData.modelId,
false
);
});
it('displays a model with an ingest pipeline and delete action is disabled', async () => {
await ml.testExecution.logTestStep('should display the model in the table');
await ml.trainedModelsTable.filterWithSearchString(modelWithPipelineData.modelId, 1);
await ml.testExecution.logTestStep('displays expected row values for the model in the table');
await ml.trainedModelsTable.assertModelsRowFields(modelWithPipelineData.modelId, {
id: modelWithPipelineData.modelId,
description: modelWithPipelineData.description,
modelTypes: modelWithPipelineData.modelTypes,
});
await ml.testExecution.logTestStep(
'should show disabled delete action for the model in the table'
);
await ml.trainedModelsTable.assertModelDeleteActionButtonEnabled(
modelWithPipelineData.modelId,
false
);
});
it('displays a model without an ingest pipeline and model can be deleted', async () => {
await ml.testExecution.logTestStep('should display the model in the table');
await ml.trainedModelsTable.filterWithSearchString(modelWithoutPipelineData.modelId, 1);
await ml.testExecution.logTestStep('displays expected row values for the model in the table');
await ml.trainedModelsTable.assertModelsRowFields(modelWithoutPipelineData.modelId, {
id: modelWithoutPipelineData.modelId,
description: modelWithoutPipelineData.description,
modelTypes: modelWithoutPipelineData.modelTypes,
});
await ml.testExecution.logTestStep(
'should show enabled delete action for the model in the table'
);
await ml.trainedModelsTable.assertModelDeleteActionButtonEnabled(
modelWithoutPipelineData.modelId,
true
);
await ml.testExecution.logTestStep('should show the delete modal');
await ml.trainedModelsTable.clickDeleteAction(modelWithoutPipelineData.modelId);
await ml.testExecution.logTestStep('should delete the model');
await ml.trainedModelsTable.confirmDeleteModel();
await ml.trainedModelsTable.assertModelDisplayedInTable(
modelWithoutPipelineData.modelId,
false
);
});
});
}

View file

@ -981,11 +981,11 @@ export function MachineLearningAPIProvider({ getService }: FtrProviderContext) {
.expect(200)
.then((res: any) => res.body);
log.debug('> Trained model crated');
log.debug('> Trained model created');
return model;
},
async createdTestTrainedModels(
async createTestTrainedModels(
modelType: ModelType,
count: number = 10,
withIngestPipelines = false

View file

@ -51,6 +51,7 @@ import { SwimLaneProvider } from './swim_lane';
import { MachineLearningDashboardJobSelectionTableProvider } from './dashboard_job_selection_table';
import { MachineLearningDashboardEmbeddablesProvider } from './dashboard_embeddables';
import { TrainedModelsProvider } from './trained_models';
import { TrainedModelsTableProvider } from './trained_models_table';
import { MachineLearningJobAnnotationsProvider } from './job_annotations_table';
export function MachineLearningProvider(context: FtrProviderContext) {
@ -121,6 +122,7 @@ export function MachineLearningProvider(context: FtrProviderContext) {
const alerting = MachineLearningAlertingProvider(context, commonUI);
const swimLane = SwimLaneProvider(context);
const trainedModels = TrainedModelsProvider(context, api, commonUI);
const trainedModelsTable = TrainedModelsTableProvider(context);
return {
anomaliesTable,
@ -168,5 +170,6 @@ export function MachineLearningProvider(context: FtrProviderContext) {
testExecution,
testResources,
trainedModels,
trainedModelsTable,
};
}

View file

@ -18,15 +18,26 @@ export function TrainedModelsProvider(
mlCommonUI: MlCommonUI
) {
const testSubjects = getService('testSubjects');
const retry = getService('retry');
return {
async createdTestTrainedModels(modelType: ModelType, count: number = 10) {
await mlApi.createdTestTrainedModels(modelType, count);
async createTestTrainedModels(
modelType: ModelType,
count: number = 10,
withIngestPipelines = false
) {
await mlApi.createTestTrainedModels(modelType, count, withIngestPipelines);
},
async assertStats(expectedTotalCount: number) {
const actualStats = await testSubjects.getVisibleText('mlInferenceModelsStatsBar');
expect(actualStats).to.eql(`Total trained models: ${expectedTotalCount}`);
await retry.tryForTime(5 * 1000, async () => {
const actualStats = await testSubjects.getVisibleText('mlInferenceModelsStatsBar');
expect(actualStats).to.eql(`Total trained models: ${expectedTotalCount}`);
});
},
async assertTableExists() {
await testSubjects.existOrFail('~mlModelsTable');
},
async assertRowsNumberPerPage(rowsNumber: 10 | 25 | 100) {

View file

@ -0,0 +1,212 @@
/*
* 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 expect from '@kbn/expect';
import { ProvidedType } from '@kbn/test';
import { WebElementWrapper } from 'test/functional/services/lib/web_element_wrapper';
import { FtrProviderContext } from '../../ftr_provider_context';
export interface TrainedModelRowData {
id: string;
description: string;
modelTypes: string[];
}
export type MlTrainedModelsTable = ProvidedType<typeof TrainedModelsTableProvider>;
export function TrainedModelsTableProvider({ getService }: FtrProviderContext) {
const testSubjects = getService('testSubjects');
const retry = getService('retry');
return new (class ModelsTable {
public async parseModelsTable() {
const table = await testSubjects.find('~mlModelsTable');
const $ = await table.parseDomContent();
const rows = [];
for (const tr of $.findTestSubjects('~mlModelsTableRow').toArray()) {
const $tr = $(tr);
const $types = $tr.findTestSubjects('mlModelType');
const modelTypes = [];
for (const el of $types.toArray()) {
modelTypes.push($(el).text().trim());
}
const rowObject: {
id: string;
description: string;
modelTypes: string[];
createdAt: string;
} = {
id: $tr
.findTestSubject('mlModelsTableColumnId')
.find('.euiTableCellContent')
.text()
.trim(),
description: $tr
.findTestSubject('mlModelsTableColumnDescription')
.find('.euiTableCellContent')
.text()
.trim(),
modelTypes,
createdAt: $tr
.findTestSubject('mlModelsTableColumnCreatedAt')
.find('.euiTableCellContent')
.text()
.trim(),
};
rows.push(rowObject);
}
return rows;
}
public rowSelector(modelId: string, subSelector?: string) {
const row = `~mlModelsTable > ~row-${modelId}`;
return !subSelector ? row : `${row} > ${subSelector}`;
}
public async waitForRefreshButtonLoaded() {
await testSubjects.existOrFail('~mlAnalyticsRefreshListButton', { timeout: 10 * 1000 });
await testSubjects.existOrFail('mlAnalyticsRefreshListButton loaded', { timeout: 30 * 1000 });
}
public async refreshModelsTable() {
await this.waitForRefreshButtonLoaded();
await testSubjects.click('~mlAnalyticsRefreshListButton');
await this.waitForRefreshButtonLoaded();
await this.waitForModelsToLoad();
}
public async waitForModelsToLoad() {
await testSubjects.existOrFail('~mlModelsTable', { timeout: 60 * 1000 });
await testSubjects.existOrFail('mlModelsTable loaded', { timeout: 30 * 1000 });
}
async getModelsSearchInput(): Promise<WebElementWrapper> {
const tableListContainer = await testSubjects.find('mlModelsTableContainer');
return await tableListContainer.findByClassName('euiFieldSearch');
}
public async assertModelsSearchInputValue(expectedSearchValue: string) {
const searchBarInput = await this.getModelsSearchInput();
const actualSearchValue = await searchBarInput.getAttribute('value');
expect(actualSearchValue).to.eql(
expectedSearchValue,
`Trained models search input value should be '${expectedSearchValue}' (got '${actualSearchValue}')`
);
}
public async filterWithSearchString(filter: string, expectedRowCount: number = 1) {
await this.waitForModelsToLoad();
const searchBarInput = await this.getModelsSearchInput();
await searchBarInput.clearValueWithKeyboard();
await searchBarInput.type(filter);
await this.assertModelsSearchInputValue(filter);
const rows = await this.parseModelsTable();
const filteredRows = rows.filter((row) => row.id === filter);
expect(filteredRows).to.have.length(
expectedRowCount,
`Filtered trained models table should have ${expectedRowCount} row(s) for filter '${filter}' (got matching items '${filteredRows}')`
);
}
public async assertModelDisplayedInTable(modelId: string, shouldBeDisplayed: boolean) {
await retry.tryForTime(5 * 1000, async () => {
await this.filterWithSearchString(modelId, shouldBeDisplayed === true ? 1 : 0);
});
}
public async assertModelsRowFields(modelId: string, expectedRow: TrainedModelRowData) {
await this.refreshModelsTable();
const rows = await this.parseModelsTable();
const modelRow = rows.filter((row) => row.id === modelId)[0];
expect(modelRow.id).to.eql(
expectedRow.id,
`Expected trained model row ID to be '${expectedRow.id}' (got '${modelRow.id}')`
);
expect(modelRow.description).to.eql(
expectedRow.description,
`Expected trained model row description to be '${expectedRow.description}' (got '${modelRow.description}')`
);
expect(modelRow.modelTypes.sort()).to.eql(
expectedRow.modelTypes.sort(),
`Expected trained model row types to be '${JSON.stringify(
expectedRow.modelTypes
)}' (got '${JSON.stringify(modelRow.modelTypes)}')`
);
// 'Created at' will be different on each run,
// so we will just assert that the value is in the expected timestamp format.
expect(modelRow.createdAt).to.match(
/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/,
`Expected trained model row created at time to have same format as '2019-12-05 12:28:34' (got '${modelRow.createdAt}')`
);
}
public async assertModelCollapsedActionsButtonExists(modelId: string, expectedValue: boolean) {
const actionsExists = await testSubjects.exists(
this.rowSelector(modelId, 'euiCollapsedItemActionsButton')
);
expect(actionsExists).to.eql(
expectedValue,
`Expected row collapsed actions menu button for trained model '${modelId}' to be ${
expectedValue ? 'visible' : 'hidden'
} (got ${actionsExists ? 'visible' : 'hidden'})`
);
}
public async assertModelDeleteActionButtonExists(modelId: string, expectedValue: boolean) {
const actionsExists = await testSubjects.exists(
this.rowSelector(modelId, 'mlModelsTableRowDeleteAction')
);
expect(actionsExists).to.eql(
expectedValue,
`Expected row delete action button for trained model '${modelId}' to be ${
expectedValue ? 'visible' : 'hidden'
} (got ${actionsExists ? 'visible' : 'hidden'})`
);
}
public async assertModelDeleteActionButtonEnabled(modelId: string, expectedValue: boolean) {
await this.assertModelDeleteActionButtonExists(modelId, true);
const isEnabled = await testSubjects.isEnabled(
this.rowSelector(modelId, 'mlModelsTableRowDeleteAction')
);
expect(isEnabled).to.eql(
expectedValue,
`Expected row delete action button for trained model '${modelId}' to be '${
expectedValue ? 'enabled' : 'disabled'
}' (got '${isEnabled ? 'enabled' : 'disabled'}')`
);
}
public async assertDeleteModalExists() {
await testSubjects.existOrFail('mlModelsDeleteModal', { timeout: 60 * 1000 });
}
public async assertDeleteModalNotExists() {
await testSubjects.missingOrFail('mlModelsDeleteModal', { timeout: 60 * 1000 });
}
public async confirmDeleteModel() {
await retry.tryForTime(30 * 1000, async () => {
await this.assertDeleteModalExists();
await testSubjects.click('mlModelsDeleteModalConfirmButton');
await this.assertDeleteModalNotExists();
});
}
public async clickDeleteAction(modelId: string) {
await testSubjects.click(this.rowSelector(modelId, 'mlModelsTableRowDeleteAction'));
await this.assertDeleteModalExists();
}
})();
}