Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Show the GPU requests and limits in workbench table #3654

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 (

Check warning on line 44 in frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx#L44

Added line #L44 was not covered by tests
<>
, {resourceValue} accelerator{Number(resourceValue) > 1 ? 's' : ''}

Check warning on line 46 in frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx#L46

Added line #L46 was not covered by tests
<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

Check warning on line 62 in frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx#L61-L62

Added lines #L61 - L62 were not covered by tests
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}>

Check warning on line 86 in frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/NotebookSizeDetails.tsx#L85-L86

Added lines #L85 - L86 were not covered by tests
<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],
};
};
Loading