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

DW: Implement "Requested resources" bullet charts on Project Metrics page (formerly "Resource usage" charts) #2673

Merged
Merged
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
35 changes: 28 additions & 7 deletions frontend/src/__mocks__/mockClusterQueueK8sResource.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import { ClusterQueueKind } from '~/k8sTypes';
import { genUID } from '~/__mocks__/mockUtils';
import { ContainerResourceAttributes } from '~/types';

type MockResourceConfigType = {
name?: string;
hasResourceGroups?: boolean;
isCpuOverQuota?: boolean;
isMemoryOverQuota?: boolean;
};

export const mockClusterQueueK8sResource = ({
name = 'test-cluster-queue',
hasResourceGroups = true,
isCpuOverQuota = false,
isMemoryOverQuota = false,
}: MockResourceConfigType): ClusterQueueKind => ({
apiVersion: 'kueue.x-k8s.io/v1beta1',
kind: 'ClusterQueue',
Expand All @@ -31,13 +36,13 @@ export const mockClusterQueueK8sResource = ({
resourceGroups: hasResourceGroups
? [
{
coveredResources: ['cpu', 'memory'],
coveredResources: [ContainerResourceAttributes.CPU, ContainerResourceAttributes.MEMORY],
flavors: [
{
name: 'test-flavor',
resources: [
{ name: 'cpu', nominalQuota: '20' },
{ name: 'memory', nominalQuota: '36Gi' },
{ name: ContainerResourceAttributes.CPU, nominalQuota: '100' },
{ name: ContainerResourceAttributes.MEMORY, nominalQuota: '64Gi' },
],
},
],
Expand All @@ -61,17 +66,33 @@ export const mockClusterQueueK8sResource = ({
{
name: 'test-flavor',
resources: [
{ borrowed: '0', name: 'cpu', total: '0' },
{ borrowed: '0', name: 'memory', total: '0' },
{
name: ContainerResourceAttributes.CPU,
borrowed: '0',
total: isCpuOverQuota ? '180' : '40',
},
{
name: ContainerResourceAttributes.MEMORY,
borrowed: '0',
total: isMemoryOverQuota ? '100Gi' : '20Gi',
},
],
},
],
flavorsUsage: [
{
name: 'test-flavor',
resources: [
{ borrowed: '0', name: 'cpu', total: '0' },
{ borrowed: '0', name: 'memory', total: '0' },
{
name: ContainerResourceAttributes.CPU,
borrowed: '0',
total: isCpuOverQuota ? '180' : '40',
},
{
name: ContainerResourceAttributes.MEMORY,
borrowed: '0',
total: isMemoryOverQuota ? '100Gi' : '20Gi',
},
],
},
],
Expand Down
27 changes: 21 additions & 6 deletions frontend/src/__mocks__/mockLocalQueueK8sResource.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import { LocalQueueKind } from '~/k8sTypes';
import { genUID } from '~/__mocks__/mockUtils';
import { ContainerResourceAttributes } from '~/types';

type MockResourceConfigType = {
name?: string;
namespace?: string;
isCpuOverQuota?: boolean;
isMemoryOverQuota?: boolean;
};

export const mockLocalQueueK8sResource = ({
name = 'test-local-queue',
namespace = 'test-project',
isCpuOverQuota = false,
isMemoryOverQuota = false,
}: MockResourceConfigType): LocalQueueKind => ({
apiVersion: 'kueue.x-k8s.io/v1beta1',
kind: 'LocalQueue',
Expand Down Expand Up @@ -39,19 +44,29 @@ export const mockLocalQueueK8sResource = ({
{
name: 'test-flavor',
resources: [
{ name: 'cpu', total: '0' },
{ name: 'memory', total: '0' },
{ name: 'nvidia.com/gpu', total: '0' },
{
name: ContainerResourceAttributes.CPU,
total: isCpuOverQuota ? '180' : '20',
},
{
name: ContainerResourceAttributes.MEMORY,
total: isMemoryOverQuota ? '100Gi' : '10Gi',
},
],
},
],
flavorsReservation: [
{
name: 'test-flavor',
resources: [
{ name: 'cpu', total: '0' },
{ name: 'memory', total: '0' },
{ name: 'nvidia.com/gpu', total: '0' },
{
name: ContainerResourceAttributes.CPU,
total: isCpuOverQuota ? '180' : '20',
},
{
name: ContainerResourceAttributes.MEMORY,
total: isMemoryOverQuota ? '100Gi' : '10Gi',
},
],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,23 +217,23 @@ describe('Project Metrics tab', () => {

cy.findByLabelText('Project metrics tab').click();

cy.findByText('Resource Usage').should('exist');
cy.findByText('Requested resources').should('exist');

cy.findByText('Top resource-consuming distributed workloads')
.closest('.dw-section-card')
.within(() => {
cy.findByText('No distributed workloads');
cy.findByText('No distributed workloads').should('exist');
});
cy.findByText('Distributed workload resource metrics')
.closest('.dw-section-card')
.within(() => {
cy.findByText('No distributed workloads');
cy.findByText('No distributed workloads').should('exist');
});
cy.findByText('Resource Usage')
cy.findByText('Requested resources')
.closest('.dw-section-card')
.within(() => {
//Resource Usage shows chart even if empty workload\
cy.findByText('Charts Placeholder');
// Requested resources shows chart even if empty workload
cy.findByTestId('requested-resources-cpu-chart-container').should('exist');
});
});

Expand All @@ -244,6 +244,14 @@ describe('Project Metrics tab', () => {
cy.findByLabelText('Project metrics tab').click();
cy.findByText('test-workload').should('exist');
});

it('Should render the requested resources charts', () => {
initIntercepts({});
globalDistributedWorkloads.visit();

cy.findByLabelText('Project metrics tab').click();
cy.findByTestId('requested-resources-cpu-chart-container').should('exist');
});
});

describe('Workload Status tab', () => {
Expand Down
39 changes: 39 additions & 0 deletions frontend/src/concepts/distributedWorkloads/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { mockClusterQueueK8sResource } from '~/__mocks__/mockClusterQueueK8sResource';
import { mockLocalQueueK8sResource } from '~/__mocks__/mockLocalQueueK8sResource';
import { mockWorkloadK8sResource } from '~/__mocks__/mockWorkloadK8sResource';
import {
getWorkloadOwnerJobName,
Expand All @@ -7,6 +9,8 @@ import {
getStatusInfo,
getWorkloadRequestedResources,
WorkloadRequestedResources,
getQueueRequestedResources,
getTotalSharedQuota,
} from '~/concepts/distributedWorkloads/utils';
import { WorkloadPodSet } from '~/k8sTypes';
import { PodContainer } from '~/types';
Expand Down Expand Up @@ -154,3 +158,38 @@ describe('getWorkloadRequestedResources', () => {
} satisfies WorkloadRequestedResources);
});
});

describe('getQueueRequestedResources', () => {
it('correctly parses and adds up requested resources from localQueues flavorsReservation', () => {
const mockLocalQueues = [
mockLocalQueueK8sResource({ name: 'test-localqueue-1' }),
mockLocalQueueK8sResource({ name: 'test-localqueue-2' }),
mockLocalQueueK8sResource({ name: 'test-localqueue-3' }),
];
expect(getQueueRequestedResources(mockLocalQueues)).toEqual({
cpuCoresRequested: 60,
memoryBytesRequested: 32212254720,
} satisfies WorkloadRequestedResources);
});

it('correctly parses and adds up requested resources from clusterQueues flavorsReservation', () => {
const mockClusterQueues = [
mockClusterQueueK8sResource({ name: 'test-clusterqueue-1' }),
mockClusterQueueK8sResource({ name: 'test-clusterqueue-2' }),
];
expect(getQueueRequestedResources(mockClusterQueues)).toEqual({
cpuCoresRequested: 80,
memoryBytesRequested: 42949672960,
} satisfies WorkloadRequestedResources);
});
});

describe('getTotalSharedQuota', () => {
it('correctly parses and adds up total resources from clusterQueue resourceGroups', () => {
const mockClusterQueue = mockClusterQueueK8sResource({});
expect(getTotalSharedQuota(mockClusterQueue)).toEqual({
cpuCoresRequested: 100,
memoryBytesRequested: 68719476736,
} satisfies WorkloadRequestedResources);
});
});
54 changes: 53 additions & 1 deletion frontend/src/concepts/distributedWorkloads/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
chart_color_green_300 as chartColorGreen,
chart_color_red_100 as chartColorRed,
} from '@patternfly/react-tokens';
import { WorkloadCondition, WorkloadKind } from '~/k8sTypes';
import { ClusterQueueKind, LocalQueueKind, WorkloadCondition, WorkloadKind } from '~/k8sTypes';
import { ContainerResourceAttributes } from '~/types';
import {
CPU_UNITS,
Expand Down Expand Up @@ -183,3 +183,55 @@ export const getWorkloadRequestedResources = (
),
};
};

export const getQueueRequestedResources = (
queues: (LocalQueueKind | ClusterQueueKind | undefined)[],
): WorkloadRequestedResources => {
const sumFromFlavorsReservation = (units: UnitOption[], attribute: ContainerResourceAttributes) =>
queues.reduce(
(queuesTotal, queue) =>
queuesTotal +
(queue?.status?.flavorsReservation || []).reduce((flavorsTotal, flavor) => {
const [value, unit] = convertToUnit(
String(flavor.resources.find(({ name }) => name === attribute)?.total || 0),
units,
'',
);
return unit.unit === '' ? flavorsTotal + value : flavorsTotal;
}, 0),
0,
);
return {
cpuCoresRequested: sumFromFlavorsReservation(CPU_UNITS, ContainerResourceAttributes.CPU),
memoryBytesRequested: sumFromFlavorsReservation(
MEMORY_UNITS_FOR_PARSING,
ContainerResourceAttributes.MEMORY,
),
};
};

export const getTotalSharedQuota = (
clusterQueue?: ClusterQueueKind,
): WorkloadRequestedResources => {
const sumFromResourceGroups = (units: UnitOption[], attribute: ContainerResourceAttributes) =>
(clusterQueue?.spec.resourceGroups || []).reduce(
(resourceGroupsTotal, resourceGroup) =>
resourceGroupsTotal +
resourceGroup.flavors.reduce((flavorsTotal, flavor) => {
const [value, unit] = convertToUnit(
String(flavor.resources.find(({ name }) => name === attribute)?.nominalQuota || 0),
units,
'',
);
return unit.unit === '' ? flavorsTotal + value : flavorsTotal;
}, 0),
0,
);
return {
cpuCoresRequested: sumFromResourceGroups(CPU_UNITS, ContainerResourceAttributes.CPU),
memoryBytesRequested: sumFromResourceGroups(
MEMORY_UNITS_FOR_PARSING,
ContainerResourceAttributes.MEMORY,
),
};
};
9 changes: 5 additions & 4 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ImageStreamStatusTagItem,
ImageStreamStatusTagCondition,
VolumeMount,
ContainerResourceAttributes,
} from './types';
import { ServingRuntimeSize } from './pages/modelServing/screens/types';

Expand Down Expand Up @@ -616,7 +617,7 @@ export type DSPipelineKind = K8sResourceCommon & {
type ClusterQueueFlavorUsage = {
name: string;
resources: {
name: string;
name: ContainerResourceAttributes;
borrowed?: string | number;
total?: string | number;
}[];
Expand Down Expand Up @@ -647,11 +648,11 @@ export type ClusterQueueKind = K8sResourceCommon & {
};
queueingStrategy?: 'StrictFIFO' | 'BestEffortFIFO';
resourceGroups?: {
coveredResources: string[];
coveredResources: ContainerResourceAttributes[];
flavors: {
name: string;
resources: {
name: string;
name: ContainerResourceAttributes;
nominalQuota: string | number; // e.g. 9 for cpu/pods, "36Gi" for memory
}[];
}[];
Expand Down Expand Up @@ -685,7 +686,7 @@ export type ClusterQueueKind = K8sResourceCommon & {
type LocalQueueFlavorUsage = {
name: string;
resources: {
name: string;
name: ContainerResourceAttributes;
total?: string | number;
}[];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ import {
StackItem,
} from '@patternfly/react-core';
import { WrenchIcon } from '@patternfly/react-icons';
import { useUser } from '~/redux/selectors';
import { DistributedWorkloadsContext } from '~/concepts/distributedWorkloads/DistributedWorkloadsContext';
import EmptyStateErrorMessage from '~/components/EmptyStateErrorMessage';
import { ResourceUsage } from './sections/ResourceUsage';
import { RequestedResources } from './sections/RequestedResources';
import { TopResourceConsumingWorkloads } from './sections/TopResourceConsumingWorkloads';
import { WorkloadResourceMetricsTable } from './sections/WorkloadResourceMetricsTable';
import { DWSectionCard } from './sections/DWSectionCard';

const GlobalDistributedWorkloadsProjectMetricsTab: React.FC = () => {
const { isAdmin } = useUser();

const { clusterQueue, localQueues } = React.useContext(DistributedWorkloadsContext);
const requiredFetches = [clusterQueue, localQueues];
const error = requiredFetches.find((f) => !!f.error)?.error;
Expand Down Expand Up @@ -58,7 +61,15 @@ const GlobalDistributedWorkloadsProjectMetricsTab: React.FC = () => {
<>
<Stack hasGutter>
<StackItem>
<DWSectionCard title="Resource Usage" content={<ResourceUsage />} />
<DWSectionCard
title="Requested resources"
helpTooltip={
isAdmin
? undefined
: 'In this section, all projects refers to all of the projects that share the specified resource. You might not have access to all of these projects.'
}
Comment on lines +67 to +70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being an admin does not mean you get access to all projects -- the two are separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah... I apologize, I did learn that the hard way once already and forgot when I wrote this originally 😕 I imagine we should just show this icon/message for all users. Even if a cluster admin sees it that seems fine to me

content={<RequestedResources />}
/>
</StackItem>
<StackItem>
<DWSectionCard
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import * as React from 'react';
import { Card, CardTitle, CardHeader, Divider } from '@patternfly/react-core';
import DashboardHelpTooltip from '~/concepts/dashboard/DashboardHelpTooltip';

export const DWSectionCard: React.FC<{
title: string;
helpTooltip?: string;
hasDivider?: boolean;
content: React.ReactNode;
}> = ({ title, hasDivider = true, content }) => (
}> = ({ title, hasDivider = true, helpTooltip, content }) => (
<Card isFullHeight className="dw-section-card">
<CardHeader>
<CardTitle>{title}</CardTitle>
<CardTitle>
{title} {helpTooltip ? <DashboardHelpTooltip content={helpTooltip} /> : null}
</CardTitle>
</CardHeader>
{hasDivider && <Divider />}
{content}
Expand Down
Loading
Loading