From 2522c0fb6c899068c9c1294533b562bb0293f7cb Mon Sep 17 00:00:00 2001 From: Lucas Fernandez Date: Thu, 11 Jan 2024 17:54:47 +0100 Subject: [PATCH] Fix performance issues in Model Serving Global --- .../e2e/modelServing/ModelServingGlobal.cy.ts | 30 ++++++++++++++ .../cypress/cypress/pages/modelServing.ts | 11 +++++ frontend/src/pages/ApplicationsPage.tsx | 16 +++++--- .../modelServing/ModelServingContext.tsx | 8 +++- .../screens/global/ModelServingGlobal.tsx | 20 +++++++++ .../screens/global/ModelServingLoading.tsx | 41 +++++++++++++++++++ .../modelServing/useInferenceServices.ts | 24 +++++++++-- .../pages/modelServing/useServingRuntimes.ts | 22 +++++++--- 8 files changed, 156 insertions(+), 16 deletions(-) create mode 100644 frontend/src/pages/modelServing/screens/global/ModelServingLoading.tsx diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelServing/ModelServingGlobal.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelServing/ModelServingGlobal.cy.ts index c7361f2405..5176f381a7 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/modelServing/ModelServingGlobal.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelServing/ModelServingGlobal.cy.ts @@ -27,6 +27,7 @@ type HandlersProps = { projectEnableModelMesh?: boolean; servingRuntimes?: ServingRuntimeKind[]; inferenceServices?: InferenceServiceKind[]; + delayInferenceServices?: boolean; }; const initIntercepts = ({ @@ -35,6 +36,7 @@ const initIntercepts = ({ projectEnableModelMesh, servingRuntimes = [mockServingRuntimeK8sResource({})], inferenceServices = [mockInferenceServiceK8sResource({})], + delayInferenceServices, }: HandlersProps) => { cy.intercept( '/api/dsc/status', @@ -78,6 +80,13 @@ const initIntercepts = ({ }, mockK8sResourceList(servingRuntimes), ); + cy.intercept( + { + method: 'GET', + pathname: '/api/k8s/apis/serving.kserve.io/v1alpha1/servingruntimes', + }, + mockK8sResourceList(servingRuntimes), + ); cy.intercept( { method: 'GET', @@ -85,6 +94,16 @@ const initIntercepts = ({ }, mockK8sResourceList(inferenceServices), ); + cy.intercept( + { + method: 'GET', + pathname: '/api/k8s/apis/serving.kserve.io/v1beta1/inferenceservices', + }, + { + delay: delayInferenceServices ? 1000 : 0, + body: mockK8sResourceList(inferenceServices), + }, + ); cy.intercept( { method: 'POST', @@ -205,6 +224,17 @@ describe('Model Serving Global', () => { inferenceServiceModal.findSubmitButton().should('be.disabled'); }); + it('All projects loading', () => { + initIntercepts({ delayInferenceServices: true, servingRuntimes: [], inferenceServices: [] }); + + // Visit the all-projects view (no project name passed here) + modelServingGlobal.visit(); + + modelServingGlobal.shouldWaitAndCancel(); + + modelServingGlobal.shouldBeEmpty(); + }); + it('Empty State No Project Selected', () => { initIntercepts({ inferenceServices: [] }); diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts index 76eae09361..512491ddcb 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts @@ -24,6 +24,17 @@ class ModelServingGlobal { return this; } + shouldWaitAndCancel() { + cy.findAllByText( + 'Retrieving model data from all projects in the cluster. This can take a few minutes.', + ); + this.findCancelButton().click(); + } + + findCancelButton() { + return cy.findByRole('button', { name: 'Cancel' }); + } + findDeployModelButton() { return cy.findByRole('button', { name: 'Deploy model' }); } diff --git a/frontend/src/pages/ApplicationsPage.tsx b/frontend/src/pages/ApplicationsPage.tsx index 2d9965de99..50f02abeb3 100644 --- a/frontend/src/pages/ApplicationsPage.tsx +++ b/frontend/src/pages/ApplicationsPage.tsx @@ -33,6 +33,7 @@ type ApplicationsPageProps = { headerAction?: React.ReactNode; headerContent?: React.ReactNode; provideChildrenPadding?: boolean; + loadingContent?: React.ReactNode; }; const ApplicationsPage: React.FC = ({ @@ -50,6 +51,7 @@ const ApplicationsPage: React.FC = ({ headerAction, headerContent, provideChildrenPadding, + loadingContent, }) => { const renderHeader = () => ( @@ -92,12 +94,14 @@ const ApplicationsPage: React.FC = ({ if (!loaded) { return ( - - - - - - + loadingContent || ( + + + + + + + ) ); } diff --git a/frontend/src/pages/modelServing/ModelServingContext.tsx b/frontend/src/pages/modelServing/ModelServingContext.tsx index 99d90a3e47..9686b22de8 100644 --- a/frontend/src/pages/modelServing/ModelServingContext.tsx +++ b/frontend/src/pages/modelServing/ModelServingContext.tsx @@ -36,6 +36,8 @@ type ModelServingContextType = { servingRuntimes: ContextResourceData; inferenceServices: ContextResourceData; project: ProjectKind | null; + preferredProject: ProjectKind | null; + projects: ProjectKind[] | null; }; type ModelServingContextProviderProps = { @@ -52,6 +54,8 @@ export const ModelServingContext = React.createContext( servingRuntimes: DEFAULT_CONTEXT_DATA, inferenceServices: DEFAULT_CONTEXT_DATA, project: null, + preferredProject: null, + projects: null, }); const ModelServingContextProvider = conditionalArea( @@ -60,7 +64,7 @@ const ModelServingContextProvider = conditionalArea { const { dashboardNamespace } = useDashboardNamespace(); const navigate = useNavigate(); - const { projects } = React.useContext(ProjectsContext); + const { projects, preferredProject } = React.useContext(ProjectsContext); const project = projects.find(byName(namespace)) ?? null; useSyncPreferredProject(project); const servingRuntimeTemplates = useContextResourceData( @@ -144,6 +148,8 @@ const ModelServingContextProvider = conditionalArea {children} diff --git a/frontend/src/pages/modelServing/screens/global/ModelServingGlobal.tsx b/frontend/src/pages/modelServing/screens/global/ModelServingGlobal.tsx index d9fc98f7f6..4e4e3f339a 100644 --- a/frontend/src/pages/modelServing/screens/global/ModelServingGlobal.tsx +++ b/frontend/src/pages/modelServing/screens/global/ModelServingGlobal.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { useNavigate } from 'react-router'; import ApplicationsPage from '~/pages/ApplicationsPage'; import { ModelServingContext } from '~/pages/modelServing/ModelServingContext'; import useServingPlatformStatuses from '~/pages/modelServing/useServingPlatformStatuses'; @@ -6,14 +7,19 @@ import { getProjectModelServingPlatform } from '~/pages/modelServing/screens/pro import EmptyModelServing from './EmptyModelServing'; import InferenceServiceListView from './InferenceServiceListView'; import ModelServingProjectSelection from './ModelServingProjectSelection'; +import ModelServingLoading from './ModelServingLoading'; const ModelServingGlobal: React.FC = () => { const { servingRuntimes: { data: servingRuntimes, loaded: servingRuntimesLoaded }, inferenceServices: { data: inferenceServices, loaded: inferenceServicesLoaded }, project: currentProject, + preferredProject, + projects, } = React.useContext(ModelServingContext); + const navigate = useNavigate(); + const servingPlatformStatuses = useServingPlatformStatuses(); const { error: notInstalledError } = getProjectModelServingPlatform( currentProject, @@ -34,6 +40,20 @@ const ModelServingGlobal: React.FC = () => { /> } provideChildrenPadding + loadingContent={ + currentProject ? undefined : ( + { + const redirectProject = preferredProject ?? projects?.[0]; + if (redirectProject) { + navigate(`/modelServing/${redirectProject?.metadata.name}`); + } + }} + /> + ) + } > void; +}; + +const ModelServingLoading: React.FC = ({ + title, + description, + onCancel, +}) => ( + + + + + {description} + + + + + + + +); + +export default ModelServingLoading; diff --git a/frontend/src/pages/modelServing/useInferenceServices.ts b/frontend/src/pages/modelServing/useInferenceServices.ts index 82d428f8ce..9efbba4251 100644 --- a/frontend/src/pages/modelServing/useInferenceServices.ts +++ b/frontend/src/pages/modelServing/useInferenceServices.ts @@ -1,20 +1,36 @@ import * as React from 'react'; -import { getInferenceServiceContext } from '~/api'; -import { InferenceServiceKind } from '~/k8sTypes'; +import { getInferenceServiceContext, listInferenceService, useAccessReview } from '~/api'; +import { AccessReviewResourceAttributes, InferenceServiceKind } from '~/k8sTypes'; import useFetchState, { FetchState, NotReadyError } from '~/utilities/useFetchState'; import useModelServingEnabled from '~/pages/modelServing/useModelServingEnabled'; import { LABEL_SELECTOR_DASHBOARD_RESOURCE } from '~/const'; +const accessReviewResource: AccessReviewResourceAttributes = { + group: 'serving.kserve.io', + resource: 'inferenceservices', + verb: 'list', +}; + const useInferenceServices = (namespace?: string): FetchState => { const modelServingEnabled = useModelServingEnabled(); + const [allowCreate, rbacLoaded] = useAccessReview({ + ...accessReviewResource, + }); + const getServingInferences = React.useCallback(() => { if (!modelServingEnabled) { return Promise.reject(new NotReadyError('Model serving is not enabled')); } - return getInferenceServiceContext(namespace, LABEL_SELECTOR_DASHBOARD_RESOURCE); - }, [namespace, modelServingEnabled]); + if (!rbacLoaded) { + return Promise.reject(new NotReadyError('Fetch is not ready')); + } + + const getInferenceServices = allowCreate ? listInferenceService : getInferenceServiceContext; + + return getInferenceServices(namespace, LABEL_SELECTOR_DASHBOARD_RESOURCE); + }, [namespace, modelServingEnabled, rbacLoaded, allowCreate]); return useFetchState(getServingInferences, [], { initialPromisePurity: true, diff --git a/frontend/src/pages/modelServing/useServingRuntimes.ts b/frontend/src/pages/modelServing/useServingRuntimes.ts index 3afa3f775b..750c2e5508 100644 --- a/frontend/src/pages/modelServing/useServingRuntimes.ts +++ b/frontend/src/pages/modelServing/useServingRuntimes.ts @@ -1,32 +1,44 @@ import * as React from 'react'; -import { getServingRuntimeContext } from '~/api'; -import { ServingRuntimeKind } from '~/k8sTypes'; +import { getServingRuntimeContext, listServingRuntimes, useAccessReview } from '~/api'; +import { AccessReviewResourceAttributes, ServingRuntimeKind } from '~/k8sTypes'; import useModelServingEnabled from '~/pages/modelServing/useModelServingEnabled'; import useFetchState, { FetchState, NotReadyError } from '~/utilities/useFetchState'; import { LABEL_SELECTOR_DASHBOARD_RESOURCE } from '~/const'; +const accessReviewResource: AccessReviewResourceAttributes = { + group: 'serving.kserve.io', + resource: 'servingruntimes', + verb: 'list', +}; + const useServingRuntimes = ( namespace?: string, notReady?: boolean, ): FetchState => { const modelServingEnabled = useModelServingEnabled(); + const [allowCreate, rbacLoaded] = useAccessReview({ + ...accessReviewResource, + }); + const getServingRuntimes = React.useCallback(() => { if (!modelServingEnabled) { return Promise.reject(new NotReadyError('Model serving is not enabled')); } - if (notReady) { + if (notReady || !rbacLoaded) { return Promise.reject(new NotReadyError('Fetch is not ready')); } - return getServingRuntimeContext(namespace, LABEL_SELECTOR_DASHBOARD_RESOURCE).catch((e) => { + const getServingRuntimes = allowCreate ? listServingRuntimes : getServingRuntimeContext; + + return getServingRuntimes(namespace, LABEL_SELECTOR_DASHBOARD_RESOURCE).catch((e) => { if (e.statusObject?.code === 404) { throw new Error('Model serving is not properly configured.'); } throw e; }); - }, [namespace, modelServingEnabled, notReady]); + }, [namespace, modelServingEnabled, notReady, rbacLoaded, allowCreate]); return useFetchState(getServingRuntimes, [], { initialPromisePurity: true,