Skip to content

Commit

Permalink
Show the GPU requests and limits in workbench table
Browse files Browse the repository at this point in the history
  • Loading branch information
dpanshug committed Jan 20, 2025
1 parent 7ffc255 commit beb21a1
Show file tree
Hide file tree
Showing 7 changed files with 254 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand All @@ -39,32 +39,10 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
formData,
setFormData,
}) => {
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { renderHook } from '@testing-library/react';
import useAcceleratorCountWarning from '~/pages/notebookController/screens/server/useAcceleratorCountWarning';

type DetectedAccelerators = {
available: Record<string, number>;
};

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('');
});
});
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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<NotebookSizeDetailsProps> = ({ notebookSize }) => {
const NotebookSizeDetails: React.FC<NotebookSizeDetailsProps> = ({
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' : ''}
<Popover
position="right"
headerContent="Accelerator details"
bodyContent={
<Stack hasGutter>
<StackItem>
Accelerator details are used by Kubernetes to schedule the workload pod on the
accelerator nodes.
</StackItem>
<StackItem>
<DescriptionList isCompact isHorizontal>
<DescriptionListGroup>
<DescriptionListTerm>Identifier</DescriptionListTerm>
<DescriptionListDescription>
{acceleratorResources.identifier && (
<ClipboardCopy
hoverTip="Copy"
clickTip="Copied"
variant="inline-compact"
data-testid="identifier-clipboard-copy"
>
{acceleratorResources.identifier}
</ClipboardCopy>
)}
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
</StackItem>
</Stack>
}
>
<>
<DashboardPopupIconButton
data-testid="accelerator-details-icon-button"
icon={<OutlinedQuestionCircleIcon />}
aria-label="More info"
style={{ paddingTop: 0, paddingBottom: 0 }}
/>
{acceleratorCountWarning && (
<Tooltip content={acceleratorCountWarning}>
<Icon status="warning">
<ExclamationTriangleIcon />
</Icon>
</Tooltip>
)}
</>
</Popover>
</>
);
};

return (
<DescriptionList>
<DescriptionListGroup>
<DescriptionListTerm>Limits</DescriptionListTerm>
<DescriptionListDescription>
{limits?.cpu ?? 'Unknown'} CPU, {formatMemory(limits?.memory) || 'Unknown'} Memory
{renderAcceleratorResource(acceleratorResources.limits)}
</DescriptionListDescription>
</DescriptionListGroup>
<DescriptionListGroup>
<DescriptionListTerm>Requests</DescriptionListTerm>
<DescriptionListDescription>
{requests?.cpu ?? 'Unknown'} CPU, {formatMemory(requests?.memory) || 'Unknown'} Memory
{renderAcceleratorResource(acceleratorResources.requests)}
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -52,6 +53,10 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
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 ?? {
Expand Down Expand Up @@ -234,7 +239,10 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
</Td>
<Td dataLabel="Limits">
<ExpandableRowContent>
<NotebookSizeDetails notebookSize={notebookSize || lastDeployedSize} />
<NotebookSizeDetails
notebookSize={notebookSize || lastDeployedSize}
acceleratorResources={acceleratorResources}
/>
</ExpandableRowContent>
</Td>
<Td />
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
});
});
});
23 changes: 23 additions & 0 deletions frontend/src/pages/projects/screens/detail/notebooks/utils.ts
Original file line number Diff line number Diff line change
@@ -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],
};
};

0 comments on commit beb21a1

Please sign in to comment.