diff --git a/frontend/src/pages/notebookController/screens/server/AcceleratorProfileSelectField.tsx b/frontend/src/pages/notebookController/screens/server/AcceleratorProfileSelectField.tsx index b96c3e94b6..174b0e79cf 100644 --- a/frontend/src/pages/notebookController/screens/server/AcceleratorProfileSelectField.tsx +++ b/frontend/src/pages/notebookController/screens/server/AcceleratorProfileSelectField.tsx @@ -20,7 +20,7 @@ import SimpleSelect, { SimpleSelectOption } from '~/components/SimpleSelect'; import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import { AcceleratorProfileFormData } from '~/utilities/useAcceleratorProfileFormState'; import { AcceleratorProfileState } from '~/utilities/useReadAcceleratorState'; -import useDetectedAccelerators from './useDetectedAccelerators'; +import useAcceleratorCountWarning from './useAcceleratorCountWarning'; type AcceleratorProfileSelectFieldProps = { supportedAcceleratorProfiles?: string[]; @@ -39,32 +39,10 @@ const AcceleratorProfileSelectField: React.FC { - const [detectedAccelerators] = useDetectedAccelerators(); - - const generateAcceleratorCountWarning = (newSize: number) => { - if (!formData.profile) { - return ''; - } - - const { identifier } = formData.profile.spec; - - const detectedAcceleratorCount = Object.entries(detectedAccelerators.available).find( - ([id]) => identifier === id, - )?.[1]; - - if (detectedAcceleratorCount === undefined) { - return `No accelerator detected with the identifier ${identifier}.`; - } - if (newSize > detectedAcceleratorCount) { - return `Only ${detectedAcceleratorCount} accelerator${ - detectedAcceleratorCount > 1 ? 's' : '' - } detected.`; - } - - return ''; - }; - - const acceleratorCountWarning = generateAcceleratorCountWarning(formData.count); + const acceleratorCountWarning = useAcceleratorCountWarning( + formData.count, + formData.profile?.spec.identifier, + ); const isAcceleratorProfileSupported = (cr: AcceleratorProfileKind) => supportedAcceleratorProfiles?.includes(cr.spec.identifier); diff --git a/frontend/src/pages/notebookController/screens/server/__tests__/useAcceleratorCountWarning.spec.ts b/frontend/src/pages/notebookController/screens/server/__tests__/useAcceleratorCountWarning.spec.ts new file mode 100644 index 0000000000..77105cc139 --- /dev/null +++ b/frontend/src/pages/notebookController/screens/server/__tests__/useAcceleratorCountWarning.spec.ts @@ -0,0 +1,52 @@ +import { renderHook } from '@testing-library/react'; +import useAcceleratorCountWarning from '~/pages/notebookController/screens/server/useAcceleratorCountWarning'; + +type DetectedAccelerators = { + available: Record; +}; + +jest.mock('../useDetectedAccelerators', () => ({ + __esModule: true, + default: jest.fn(), +})); + +describe('useAcceleratorCountWarning', () => { + const mockUseDetectedAccelerators = require('../useDetectedAccelerators').default as jest.Mock< + DetectedAccelerators[] + >; + + beforeEach(() => { + mockUseDetectedAccelerators.mockClear(); + }); + + it('should return an empty string if identifier is not provided', () => { + mockUseDetectedAccelerators.mockReturnValue([{ available: {} }]); + + const { result } = renderHook(() => useAcceleratorCountWarning()); + expect(result.current).toBe(''); + }); + + it('should return a message if no accelerator is detected for the given identifier', () => { + mockUseDetectedAccelerators.mockReturnValue([{ available: {} }]); + + const { result } = renderHook(() => useAcceleratorCountWarning(undefined, 'test-id')); + expect(result.current).toBe('No accelerator detected with the identifier test-id.'); + }); + + it('should return a message if newSize is greater than detected accelerator count', () => { + mockUseDetectedAccelerators.mockReturnValue([{ available: { 'test-id': 2 } }]); + + const { result } = renderHook(() => useAcceleratorCountWarning(3, 'test-id')); + expect(result.current).toBe('Only 2 accelerators detected.'); + }); + + it('should return an empty string if newSize is less than or equal to detected accelerator count', () => { + mockUseDetectedAccelerators.mockReturnValue([{ available: { 'test-id': 2 } }]); + + const { result: result1 } = renderHook(() => useAcceleratorCountWarning(2, 'test-id')); + expect(result1.current).toBe(''); + + const { result: result2 } = renderHook(() => useAcceleratorCountWarning(1, 'test-id')); + expect(result2.current).toBe(''); + }); +}); diff --git a/frontend/src/pages/notebookController/screens/server/useAcceleratorCountWarning.tsx b/frontend/src/pages/notebookController/screens/server/useAcceleratorCountWarning.tsx new file mode 100644 index 0000000000..a70f869619 --- /dev/null +++ b/frontend/src/pages/notebookController/screens/server/useAcceleratorCountWarning.tsx @@ -0,0 +1,27 @@ +import useDetectedAccelerators from './useDetectedAccelerators'; + +const useAcceleratorCountWarning = (newSize?: number | string, identifier?: string): string => { + const [detectedAccelerators] = useDetectedAccelerators(); + + if (!identifier) { + return ''; + } + + const detectedAcceleratorCount = Object.entries(detectedAccelerators.available).find( + ([id]) => identifier === id, + )?.[1]; + + if (detectedAcceleratorCount === undefined) { + return `No accelerator detected with the identifier ${identifier}.`; + } + + if (newSize !== undefined && Number(newSize) > detectedAcceleratorCount) { + return `Only ${detectedAcceleratorCount} accelerator${ + detectedAcceleratorCount > 1 ? 's' : '' + } detected.`; + } + + return ''; +}; + +export default useAcceleratorCountWarning; diff --git a/frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx b/frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx index fe7baa8c61..28caaf86ea 100644 --- a/frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx +++ b/frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx @@ -1,34 +1,114 @@ import * as React from 'react'; import { + ClipboardCopy, DescriptionList, DescriptionListDescription, DescriptionListGroup, DescriptionListTerm, + Icon, + Popover, + Stack, + StackItem, + Tooltip, } from '@patternfly/react-core'; +import { ExclamationTriangleIcon, OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import { NotebookSize } from '~/types'; import { formatMemory } from '~/utilities/valueUnits'; +import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; +import useAcceleratorCountWarning from '~/pages/notebookController/screens/server/useAcceleratorCountWarning'; +import { AcceleratorResources } from './utils'; type NotebookSizeDetailsProps = { notebookSize: NotebookSize; + acceleratorResources: AcceleratorResources; }; -const NotebookSizeDetails: React.FC = ({ notebookSize }) => { +const NotebookSizeDetails: React.FC = ({ + notebookSize, + acceleratorResources, +}) => { const { resources: { requests, limits }, } = notebookSize; + const acceleratorCountWarning = useAcceleratorCountWarning( + acceleratorResources.requests, + acceleratorResources.identifier, + ); + + const renderAcceleratorResource = (resourceValue?: number | string) => { + if (!resourceValue) { + return null; + } + + return ( + <> + , {resourceValue} accelerator{Number(resourceValue) > 1 ? 's' : ''} + + + Accelerator details are used by Kubernetes to schedule the workload pod on the + accelerator nodes. + + + + + Identifier + + {acceleratorResources.identifier && ( + + {acceleratorResources.identifier} + + )} + + + + + + } + > + <> + } + aria-label="More info" + style={{ paddingTop: 0, paddingBottom: 0 }} + /> + {acceleratorCountWarning && ( + + + + + + )} + + + + ); + }; + return ( Limits {limits?.cpu ?? 'Unknown'} CPU, {formatMemory(limits?.memory) || 'Unknown'} Memory + {renderAcceleratorResource(acceleratorResources.limits)} Requests {requests?.cpu ?? 'Unknown'} CPU, {formatMemory(requests?.memory) || 'Unknown'} Memory + {renderAcceleratorResource(acceleratorResources.requests)} diff --git a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx index bc3a39efc6..3883a8b9c9 100644 --- a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx +++ b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx @@ -30,6 +30,7 @@ import NotebookSizeDetails from './NotebookSizeDetails'; import useNotebookImage from './useNotebookImage'; import useNotebookDeploymentSize from './useNotebookDeploymentSize'; import useNotebookAcceleratorProfileFormState from './useNotebookAcceleratorProfileFormState'; +import { extractAcceleratorResources } from './utils'; type NotebookTableRowProps = { obj: NotebookState; @@ -52,6 +53,10 @@ const NotebookTableRow: React.FC = ({ const navigate = useNavigate(); const [isExpanded, setExpanded] = React.useState(false); const { size: notebookSize } = useNotebookDeploymentSize(obj.notebook); + const acceleratorResources = extractAcceleratorResources( + obj.notebook.spec.template.spec.containers[0].resources, + ); + const lastDeployedSize: NotebookSize = { name: 'Custom', resources: obj.notebook.spec.template.spec.containers[0].resources ?? { @@ -234,7 +239,10 @@ const NotebookTableRow: React.FC = ({ - + diff --git a/frontend/src/pages/projects/screens/detail/notebooks/__tests__/utils.spec.ts b/frontend/src/pages/projects/screens/detail/notebooks/__tests__/utils.spec.ts new file mode 100644 index 0000000000..6e361e5ce2 --- /dev/null +++ b/frontend/src/pages/projects/screens/detail/notebooks/__tests__/utils.spec.ts @@ -0,0 +1,57 @@ +import { ContainerResources } from '~/types'; +import { extractAcceleratorResources } from '~/pages/projects/screens/detail/notebooks/utils'; + +describe('extractAcceleratorResources', () => { + it('should return empty object if no resources are provided', () => { + const result = extractAcceleratorResources(); + expect(result).toEqual({}); + }); + + it('should return empty object if resources do not contain accelerators', () => { + const resources: ContainerResources = { + limits: { cpu: '1000m', memory: '2Gi' }, + requests: { cpu: '500m', memory: '1Gi' }, + }; + const result = extractAcceleratorResources(resources); + expect(result).toEqual({}); + }); + + it('should extract accelerator resources from limits', () => { + const resources: ContainerResources = { + limits: { cpu: '1000m', memory: '2Gi', 'nvidia.com/gpu': '1' }, + requests: { cpu: '500m', memory: '1Gi' }, + }; + const result = extractAcceleratorResources(resources); + expect(result).toEqual({ + limits: '1', + requests: undefined, + identifier: 'nvidia.com/gpu', + }); + }); + + it('should extract accelerator resources from requests', () => { + const resources: ContainerResources = { + limits: { cpu: '1000m', memory: '2Gi' }, + requests: { cpu: '500m', memory: '1Gi', 'nvidia.com/gpu': '1' }, + }; + const result = extractAcceleratorResources(resources); + expect(result).toEqual({ + limits: undefined, + requests: '1', + identifier: 'nvidia.com/gpu', + }); + }); + + it('should extract accelerator resources from both limits and requests', () => { + const resources: ContainerResources = { + limits: { cpu: '1000m', memory: '2Gi', 'nvidia.com/gpu': '2' }, + requests: { cpu: '500m', memory: '1Gi', 'nvidia.com/gpu': '1' }, + }; + const result = extractAcceleratorResources(resources); + expect(result).toEqual({ + limits: '2', + requests: '1', + identifier: 'nvidia.com/gpu', + }); + }); +}); diff --git a/frontend/src/pages/projects/screens/detail/notebooks/utils.ts b/frontend/src/pages/projects/screens/detail/notebooks/utils.ts new file mode 100644 index 0000000000..0dd393fb92 --- /dev/null +++ b/frontend/src/pages/projects/screens/detail/notebooks/utils.ts @@ -0,0 +1,23 @@ +import { ContainerResources } from '~/types'; + +export type AcceleratorResources = { + limits?: number | string; + requests?: number | string; + identifier?: string; +}; + +export const extractAcceleratorResources = ( + resources?: ContainerResources, +): AcceleratorResources => { + const findAcceleratorResource = (res?: { [key: string]: number | string | undefined }) => + Object.entries(res || {}).find(([key]) => !['cpu', 'memory'].includes(key)); + + const limitsResource = findAcceleratorResource(resources?.limits); + const requestsResource = findAcceleratorResource(resources?.requests); + + return { + limits: limitsResource?.[1], + requests: requestsResource?.[1], + identifier: limitsResource?.[0] || requestsResource?.[0], + }; +};