Skip to content

Commit

Permalink
change node resources table based per requested
Browse files Browse the repository at this point in the history
  • Loading branch information
DaoDaoNoCode committed Jan 10, 2025
1 parent 9c96366 commit 1005447
Show file tree
Hide file tree
Showing 13 changed files with 213 additions and 129 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ class ManageHardwareProfile {
return cy.findByTestId('hardware-profile-node-resources-table');
}

findNodeResourceTableAlert() {
return cy.findByTestId('node-resource-table-alert');
}

getTolerationTableRow(name: string) {
return new TolerationRow(() =>
this.findTolerationTable().find(`[data-label=Key]`).contains(name).parents('tr'),
Expand Down Expand Up @@ -357,6 +361,10 @@ class NodeResourceModal extends Modal {
return this.find().findByTestId('node-resource-identifier-input');
}

findNodeResourceTypeSelect() {
return this.find().findByTestId('node-resource-type-select');
}

findNodeResourceExistingErrorMessage() {
return this.find().findByTestId('resource-identifier-error');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,6 @@ describe('Manage Hardware Profile', () => {

// test node resource table
createHardwareProfile.findNodeResourceTable().should('exist');
// verify both CPU and Memory rows exist and cannot be deleted
createHardwareProfile
.getNodeResourceTableRow('cpu')
.findDeleteAction()
.should('have.attr', 'aria-disabled');
createHardwareProfile
.getNodeResourceTableRow('memory')
.findDeleteAction()
.should('have.attr', 'aria-disabled');
// open node resource modal
createHardwareProfile.findAddNodeResourceButton().click();
// fill in form required fields
Expand All @@ -144,15 +135,15 @@ describe('Manage Hardware Profile', () => {
createNodeResourceModal.findNodeResourceExistingErrorMessage().should('exist');
createNodeResourceModal.findNodeResourceSubmitButton().should('be.disabled');
createNodeResourceModal.findNodeResourceIdentifierInput().fill('test-gpu');
createNodeResourceModal.findNodeResourceTypeSelect().should('contain.text', 'Other');
createNodeResourceModal.findNodeResourceSubmitButton().should('be.enabled');
createNodeResourceModal.findNodeResourceSubmitButton().click();
// test that values were added correctly
createHardwareProfile.getNodeResourceTableRow('test-gpu').shouldHaveResourceLabel('Test GPU');

// test edit node resource
// test cannot edit cpu or memory identifier
createHardwareProfile.getNodeResourceTableRow('cpu').findEditAction().click();
editNodeResourceModal.findNodeResourceIdentifierInput().should('be.disabled');
editNodeResourceModal.findNodeResourceTypeSelect().should('contain.text', 'CPU');
// test default value should be within min and max value
editNodeResourceModal.selectNodeResourceDefaultUnit('Milicores');
editNodeResourceModal.findNodeResourceDefaultErrorMessage().should('exist');
Expand All @@ -166,7 +157,7 @@ describe('Manage Hardware Profile', () => {
editNodeResourceModal.findCancelButton().click();

createHardwareProfile.getNodeResourceTableRow('memory').findEditAction().click();
editNodeResourceModal.findNodeResourceIdentifierInput().should('be.disabled');
editNodeResourceModal.findNodeResourceTypeSelect().should('contain.text', 'Memory');
// test default value should be within min and max value
editNodeResourceModal.selectNodeResourceDefaultUnit('MiB');
editNodeResourceModal.findNodeResourceDefaultErrorMessage().should('exist');
Expand Down Expand Up @@ -203,6 +194,22 @@ describe('Manage Hardware Profile', () => {
.shouldHaveResourceLabel('Test GPU Edited')
.shouldHaveResourceIdentifier('test-gpu-edited');

// test deleting the last CPU trigger the alert shown
createHardwareProfile.getNodeResourceTableRow('cpu').findDeleteAction().click();
createHardwareProfile.findNodeResourceTableAlert().should('exist');
createHardwareProfile.findAddNodeResourceButton().click();
createNodeResourceModal.findNodeResourceLabelInput().fill('CPU');
createNodeResourceModal.findNodeResourceIdentifierInput().fill('cpu');
createNodeResourceModal.findNodeResourceTypeSelect().findSelectOption('CPU').click();
createNodeResourceModal.findNodeResourceSubmitButton().should('be.enabled');
createNodeResourceModal.findNodeResourceSubmitButton().click();

createHardwareProfile.findNodeResourceTableAlert().should('not.exist');

// test deleting the last Memory trigger the alert shown
createHardwareProfile.getNodeResourceTableRow('memory').findDeleteAction().click();
createHardwareProfile.findNodeResourceTableAlert().should('exist');

cy.interceptK8s(
'POST',
{
Expand All @@ -216,27 +223,21 @@ describe('Manage Hardware Profile', () => {

cy.wait('@createHardwareProfile').then((interception) => {
expect(interception.request.body.spec.identifiers).to.be.eql([
{
identifier: 'cpu',
displayName: 'CPU',
defaultCount: 2,
maxCount: 4,
minCount: 1,
},
{
identifier: 'memory',
displayName: 'Memory',
defaultCount: '4Gi',
minCount: '2Gi',
maxCount: '8Gi',
},
{
displayName: 'Test GPU Edited',
identifier: 'test-gpu-edited',
minCount: 1,
maxCount: 13,
defaultCount: 1,
},
{
identifier: 'cpu',
displayName: 'CPU',
defaultCount: 2,
maxCount: 4,
minCount: 1,
resourceType: 'CPU',
},
]);
});
});
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/pages/hardwareProfiles/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
ManageHardwareProfileSectionID,
ManageHardwareProfileSectionTitlesType,
} from '~/pages/hardwareProfiles/manage/types';
import { IdentifierResourceType } from '~/types';

export const hardwareProfileColumns: SortableData<HardwareProfileKind>[] = [
{
Expand Down Expand Up @@ -88,13 +89,15 @@ export const DEFAULT_HARDWARE_PROFILE_SPEC: HardwareProfileKind['spec'] = {
defaultCount: 2,
maxCount: 4,
minCount: 1,
resourceType: IdentifierResourceType.CPU,
},
{
identifier: 'memory',
displayName: 'Memory',
defaultCount: '4Gi',
minCount: '2Gi',
maxCount: '8Gi',
resourceType: IdentifierResourceType.MEMORY,
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import ManageNodeSelectorSection from '~/pages/hardwareProfiles/manage/ManageNod
import ManageTolerationSection from '~/pages/hardwareProfiles/manage/ManageTolerationSection';
import ManageHardwareProfileFooter from '~/pages/hardwareProfiles/manage/ManageHardwareProfileFooter';
import ManageNodeResourceSection from '~/pages/hardwareProfiles/manage/ManageNodeResourceSection';
import { isNodeResourcesSectionValid } from '~/pages/hardwareProfiles/utils';
import { HardwareProfileFormData, ManageHardwareProfileSectionID } from './types';

type ManageHardwareProfileProps = {
Expand Down Expand Up @@ -70,9 +69,7 @@ const ManageHardwareProfile: React.FC<ManageHardwareProfileProps> = ({
[state, profileNameDesc],
);

const validFormData =
isK8sNameDescriptionDataValid(profileNameDesc) &&
isNodeResourcesSectionValid(state.identifiers);
const validFormData = isK8sNameDescriptionDataValid(profileNameDesc);

return (
<ApplicationsPage
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { FormSection, Flex, FlexItem, Button } from '@patternfly/react-core';
import { Identifier } from '~/types';
import { FormSection, Flex, FlexItem, Button, Alert, AlertVariant } from '@patternfly/react-core';
import { AddCircleOIcon } from '@patternfly/react-icons';
import { Identifier, IdentifierResourceType } from '~/types';
import NodeResourceTable from '~/pages/hardwareProfiles/nodeResource/NodeResourceTable';
import ManageNodeResourceModal from '~/pages/hardwareProfiles/nodeResource/ManageNodeResourceModal';
import { ManageHardwareProfileSectionTitles } from '~/pages/hardwareProfiles/const';
Expand All @@ -16,6 +17,7 @@ const ManageNodeResourceSection: React.FC<ManageNodeResourceSectionProps> = ({
setNodeResources,
}) => {
const [isNodeResourceModalOpen, setIsNodeResourceModalOpen] = React.useState<boolean>(false);
const isEmpty = nodeResources.length === 0;
return (
<>
<FormSection
Expand All @@ -24,24 +26,47 @@ const ManageNodeResourceSection: React.FC<ManageNodeResourceSectionProps> = ({
<FlexItem>
{ManageHardwareProfileSectionTitles[ManageHardwareProfileSectionID.IDENTIFIERS]}
</FlexItem>
<FlexItem>
<Button
variant="secondary"
onClick={() => setIsNodeResourceModalOpen(true)}
data-testid="add-node-resource-button"
>
Add resource
</Button>
</FlexItem>
{!isEmpty && (
<FlexItem>
<Button
variant="secondary"
onClick={() => setIsNodeResourceModalOpen(true)}
data-testid="add-node-resource-button"
>
Add resource
</Button>
</FlexItem>
)}
</Flex>
}
>
Every hardware profile must include CPU and memory resources. Additional resources, such as
GPUs, can be added here.
<NodeResourceTable
nodeResources={nodeResources}
onUpdate={(newResources) => setNodeResources(newResources)}
/>
Every hardware profile is highly recommended to include CPU and memory resources. Additional
resources, such as GPUs, can be added here, too.
{!(
nodeResources.some(
(identifier) => identifier.resourceType === IdentifierResourceType.CPU,
) &&
nodeResources.some(
(identifier) => identifier.resourceType === IdentifierResourceType.MEMORY,
)
) && (
<Alert
title="Missing CPU or Memory node resources"
isInline
variant={AlertVariant.warning}
data-testid="node-resource-table-alert"
>
It is not recommended to remove the CPU or Memory. The resources that use this hardware
profile will schedule, but will be very unstable due to not having any lower or upper
resource bounds.
</Alert>
)}
{!isEmpty && (
<NodeResourceTable
nodeResources={nodeResources}
onUpdate={(newResources) => setNodeResources(newResources)}
/>
)}
</FormSection>
{isNodeResourceModalOpen && (
<ManageNodeResourceModal
Expand All @@ -50,6 +75,17 @@ const ManageNodeResourceSection: React.FC<ManageNodeResourceSectionProps> = ({
nodeResources={nodeResources}
/>
)}
{isEmpty && (
<Button
isInline
icon={<AddCircleOIcon />}
variant="link"
onClick={() => setIsNodeResourceModalOpen(true)}
data-testid="add-node-resource-button"
>
Add node resource
</Button>
)}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ import { FormGroup, FormHelperText, HelperText, HelperTextItem } from '@patternf
import MemoryField from '~/components/MemoryField';
import CPUField from '~/components/CPUField';
import NumberInputWrapper from '~/components/NumberInputWrapper';
import { IdentifierResourceType } from '~/types';

type CountFormFieldProps = {
label: string;
fieldId: string;
size: number | string;
setSize: (value: number | string) => void;
identifier: string;
type?: IdentifierResourceType;
errorMessage?: string;
isValid?: boolean;
};
Expand All @@ -19,15 +20,15 @@ const CountFormField: React.FC<CountFormFieldProps> = ({
fieldId,
size,
setSize,
identifier,
type,
errorMessage,
isValid = true,
}) => {
const renderInputField = () => {
switch (identifier) {
case 'cpu':
switch (type) {
case IdentifierResourceType.CPU:
return <CPUField onChange={(value) => setSize(value)} value={size} />;
case 'memory':
case IdentifierResourceType.MEMORY:
return <MemoryField onChange={(value) => setSize(value)} value={String(size)} />;
default:
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { Modal } from '@patternfly/react-core/deprecated';
import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter';
import { Identifier } from '~/types';
import { Identifier, IdentifierResourceType } from '~/types';
import useGenericObjectState from '~/utilities/useGenericObjectState';
import { CPU_UNITS, MEMORY_UNITS_FOR_SELECTION, UnitOption } from '~/utilities/valueUnits';
import { EMPTY_IDENTIFIER } from './const';
Expand Down Expand Up @@ -32,11 +32,11 @@ const ManageNodeResourceModal: React.FC<ManageNodeResourceModalProps> = ({
!nodeResources.some((i) => i.identifier === identifier.identifier);

React.useEffect(() => {
switch (identifier.identifier) {
case 'cpu':
switch (identifier.resourceType) {
case IdentifierResourceType.CPU:
setUnitOptions(CPU_UNITS);
break;
case 'memory':
case IdentifierResourceType.MEMORY:
setUnitOptions(MEMORY_UNITS_FOR_SELECTION);
break;
default:
Expand Down Expand Up @@ -74,7 +74,6 @@ const ManageNodeResourceModal: React.FC<ManageNodeResourceModalProps> = ({
identifier={identifier}
setIdentifier={setIdentifier}
unitOptions={unitOptions}
isExistingIdentifier={!!existingIdentifier}
isUniqueIdentifier={isUniqueIdentifier}
/>
</Modal>
Expand Down
Loading

0 comments on commit 1005447

Please sign in to comment.