From 284829b512845d7294d54eee3eb0e3aef03c80ce Mon Sep 17 00:00:00 2001 From: gitdallas <5322142+gitdallas@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:17:46 -0500 Subject: [PATCH] fix(MR): disable archive for mr/mv if has deploys Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> --- .../mockInferenceServiceK8sResource.ts | 2 + .../modelRegistry/modelVersionArchive.cy.ts | 27 ++++++++- .../ModelVersionDetails.tsx | 6 +- .../ModelVersionDetailsHeaderActions.tsx | 6 ++ .../screens/ModelVersions/ModelVersions.tsx | 9 ++- .../ModelVersionsHeaderActions.tsx | 12 +++- .../ModelVersions/ModelVersionsTable.tsx | 56 ++++++++++++------- .../ModelVersions/ModelVersionsTableRow.tsx | 6 ++ .../RegisteredModels/RegisteredModelTable.tsx | 47 ++++++++++------ .../RegisteredModelTableRow.tsx | 6 ++ 10 files changed, 135 insertions(+), 42 deletions(-) diff --git a/frontend/src/__mocks__/mockInferenceServiceK8sResource.ts b/frontend/src/__mocks__/mockInferenceServiceK8sResource.ts index f878a01375..5970ba6521 100644 --- a/frontend/src/__mocks__/mockInferenceServiceK8sResource.ts +++ b/frontend/src/__mocks__/mockInferenceServiceK8sResource.ts @@ -99,6 +99,8 @@ export const mockInferenceServiceK8sResource = ({ generation: 1, labels: { name, + [KnownLabels.REGISTERED_MODEL_ID]: '1', + [KnownLabels.MODEL_VERSION_ID]: '3', [KnownLabels.DASHBOARD_RESOURCE]: 'true', ...(kserveInternalLabel && { 'networking.knative.dev/visibility': 'cluster-local' }), }, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts index 03610b9aed..bb30f4eddc 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts @@ -1,8 +1,17 @@ /* eslint-disable camelcase */ -import { mockDscStatus, mockK8sResourceList } from '~/__mocks__'; +import { + mockDscStatus, + mockInferenceServiceK8sResource, + mockK8sResourceList, + mockProjectK8sResource, +} from '~/__mocks__'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; -import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models'; +import { + InferenceServiceModel, + ProjectModel, + ServiceModel, +} from '~/__tests__/cypress/cypress/utils/models'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; import type { ModelVersion } from '~/concepts/modelRegistry/types'; @@ -272,6 +281,20 @@ describe('Archiving version', () => { }); }); + it('Cannot archive version that has a deployment from versions table', () => { + cy.interceptK8sList(ProjectModel, mockK8sResourceList([mockProjectK8sResource({})])); + cy.interceptK8sList( + InferenceServiceModel, + mockK8sResourceList([mockInferenceServiceK8sResource({})]), + ); + initIntercepts({}); + + modelVersionArchive.visitModelVersionList(); + + const modelVersionRow = modelRegistry.getModelVersionRow('model version 3'); + modelVersionRow.findKebabAction('Archive model version').should('have.attr', 'aria-disabled'); + }); + it('Archive version from versions details', () => { cy.interceptOdh( 'PATCH /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/model_versions/:modelVersionId', diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx index 0e0e91053e..b25e9df8b5 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx @@ -118,7 +118,11 @@ const ModelVersionsDetails: React.FC = ({ tab, ...page /> - + 0} + refresh={refresh} + /> ) diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx index 7585ad64e7..b627c3b1f4 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx @@ -13,11 +13,13 @@ import DeployRegisteredModelModal from '~/pages/modelRegistry/screens/components interface ModelVersionsDetailsHeaderActionsProps { mv: ModelVersion; + hasDeployment?: boolean; refresh: () => void; } const ModelVersionsDetailsHeaderActions: React.FC = ({ mv, + hasDeployment = false, refresh, }) => { const { apiState } = React.useContext(ModelRegistryContext); @@ -60,10 +62,14 @@ const ModelVersionsDetailsHeaderActions: React.FC setIsArchiveModalOpen(true)} + tooltipProps={ + hasDeployment ? { content: 'Deployed versions cannot be archived' } : undefined + } ref={tooltipRef} > Archive version diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx index 5761fffaa5..7323349cfe 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx @@ -8,6 +8,8 @@ import useRegisteredModelById from '~/concepts/modelRegistry/apiHooks/useRegiste import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext'; import { ModelState } from '~/concepts/modelRegistry/types'; import { registeredModelArchiveDetailsUrl } from '~/pages/modelRegistry/screens/routeUtils'; +import { useMakeFetchObject } from '~/utilities/useMakeFetchObject'; +import useInferenceServices from '~/pages/modelServing/useInferenceServices'; import ModelVersionsTabs from './ModelVersionsTabs'; import ModelVersionsHeaderActions from './ModelVersionsHeaderActions'; import { ModelVersionsTab } from './const'; @@ -27,6 +29,7 @@ const ModelVersions: React.FC = ({ tab, ...pageProps }) => { const loadError = mvLoadError || rmLoadError; const loaded = mvLoaded && rmLoaded; const navigate = useNavigate(); + const inferenceServices = useMakeFetchObject(useInferenceServices(undefined, rmId)); useEffect(() => { if (rm?.state === ModelState.ARCHIVED) { @@ -52,7 +55,11 @@ const ModelVersions: React.FC = ({ tab, ...pageProps }) => { } title={rm?.name} - headerAction={rm && } + headerAction={ + rm && ( + + ) + } description={} loadError={loadError} loaded={loaded} diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx index 654d062283..cfa33d03ab 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx @@ -16,9 +16,13 @@ import { RegisteredModel, ModelState } from '~/concepts/modelRegistry/types'; interface ModelVersionsHeaderActionsProps { rm: RegisteredModel; + hasDeployments?: boolean; } -const ModelVersionsHeaderActions: React.FC = ({ rm }) => { +const ModelVersionsHeaderActions: React.FC = ({ + rm, + hasDeployments = false, +}) => { const { apiState } = React.useContext(ModelRegistryContext); const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); @@ -56,6 +60,12 @@ const ModelVersionsHeaderActions: React.FC = ({ key="archive-model-button" onClick={() => setIsArchiveModalOpen(true)} ref={tooltipRef} + isAriaDisabled={hasDeployments} + tooltipProps={ + hasDeployments + ? { content: 'Models with deployed versions cannot be archived.' } + : undefined + } > Archive model diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx index b70823c463..cb53691a05 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx @@ -2,8 +2,11 @@ import * as React from 'react'; import { Table } from '~/components/table'; import { ModelVersion } from '~/concepts/modelRegistry/types'; import DashboardEmptyTableView from '~/concepts/dashboard/DashboardEmptyTableView'; -import ModelVersionsTableRow from './ModelVersionsTableRow'; +import useInferenceServices from '~/pages/modelServing/useInferenceServices'; +import { useMakeFetchObject } from '~/utilities/useMakeFetchObject'; +import { KnownLabels } from '~/k8sTypes'; import { mvColumns } from './ModelVersionsTableColumns'; +import ModelVersionsTableRow from './ModelVersionsTableRow'; type ModelVersionsTableProps = { clearFilters: () => void; @@ -18,25 +21,36 @@ const ModelVersionsTable: React.FC = ({ toolbarContent, isArchiveModel, refresh, -}) => ( - } - rowRenderer={(mv) => ( - - )} - /> -); +}) => { + const inferenceServices = useMakeFetchObject( + useInferenceServices(undefined, modelVersions[0].registeredModelId), + ); + return ( +
} + rowRenderer={(mv) => { + const hasDeployment = !!inferenceServices.data.some( + (s) => s.metadata.labels?.[KnownLabels.MODEL_VERSION_ID] === mv.id, + ); + return ( + + ); + }} + /> + ); +}; export default ModelVersionsTable; diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx index 33e7dca3ca..d885f3f21b 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx @@ -21,6 +21,7 @@ type ModelVersionsTableRowProps = { modelVersion: ModelVersion; isArchiveRow?: boolean; isArchiveModel?: boolean; + hasDeployment?: boolean; refresh: () => void; }; @@ -28,6 +29,7 @@ const ModelVersionsTableRow: React.FC = ({ modelVersion: mv, isArchiveRow, isArchiveModel, + hasDeployment = false, refresh, }) => { const navigate = useNavigate(); @@ -52,6 +54,10 @@ const ModelVersionsTableRow: React.FC = ({ { title: 'Archive model version', onClick: () => setIsArchiveModalOpen(true), + isAriaDisabled: hasDeployment, + tooltipProps: hasDeployment + ? { content: 'Deployed versions cannot be archived' } + : undefined, }, ]; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx index 82092c1fce..c31f746e26 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx @@ -2,8 +2,11 @@ import * as React from 'react'; import { Table } from '~/components/table'; import { RegisteredModel } from '~/concepts/modelRegistry/types'; import DashboardEmptyTableView from '~/concepts/dashboard/DashboardEmptyTableView'; -import { rmColumns } from './RegisteredModelsTableColumns'; +import useInferenceServices from '~/pages/modelServing/useInferenceServices'; +import { useMakeFetchObject } from '~/utilities/useMakeFetchObject'; +import { KnownLabels } from '~/k8sTypes'; import RegisteredModelTableRow from './RegisteredModelTableRow'; +import { rmColumns } from './RegisteredModelsTableColumns'; type RegisteredModelTableProps = { clearFilters: () => void; @@ -16,20 +19,32 @@ const RegisteredModelTable: React.FC = ({ registeredModels, toolbarContent, refresh, -}) => ( -
} - rowRenderer={(rm) => ( - - )} - /> -); +}) => { + const inferenceServices = useMakeFetchObject(useInferenceServices()); + const hasDeploys = (rmId: string) => + !!inferenceServices.data.some( + (s) => s.metadata.labels?.[KnownLabels.REGISTERED_MODEL_ID] === rmId, + ); + return ( +
} + rowRenderer={(rm) => ( + + )} + /> + ); +}; export default RegisteredModelTable; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx index 1abdd11026..826bda0e47 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx @@ -19,12 +19,14 @@ import { ModelVersionsTab } from '~/pages/modelRegistry/screens/ModelVersions/co type RegisteredModelTableRowProps = { registeredModel: RegisteredModel; isArchiveRow?: boolean; + hasDeploys?: boolean; refresh: () => void; }; const RegisteredModelTableRow: React.FC = ({ registeredModel: rm, isArchiveRow, + hasDeploys = false, refresh, }) => { const { apiState } = React.useContext(ModelRegistryContext); @@ -57,6 +59,10 @@ const RegisteredModelTableRow: React.FC = ({ : { title: 'Archive model', onClick: () => setIsArchiveModalOpen(true), + isAriaDisabled: hasDeploys, + tooltipProps: hasDeploys + ? { content: 'Models with deployed versions cannot be archived.' } + : undefined, }, ];