diff --git a/frontend/src/__tests__/cypress/cypress/pages/hardwareProfile.ts b/frontend/src/__tests__/cypress/cypress/pages/hardwareProfile.ts index c6d427150b..dea94bfa4b 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/hardwareProfile.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/hardwareProfile.ts @@ -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'), @@ -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'); } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/hardwareProfiles/manageHardwareProfiles.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/hardwareProfiles/manageHardwareProfiles.cy.ts index ea6653dd11..eeee29b89e 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/hardwareProfiles/manageHardwareProfiles.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/hardwareProfiles/manageHardwareProfiles.cy.ts @@ -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 @@ -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'); @@ -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'); @@ -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', { @@ -216,20 +223,6 @@ 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', @@ -237,6 +230,14 @@ describe('Manage Hardware Profile', () => { maxCount: 13, defaultCount: 1, }, + { + identifier: 'cpu', + displayName: 'CPU', + defaultCount: 2, + maxCount: 4, + minCount: 1, + resourceType: 'CPU', + }, ]); }); }); diff --git a/frontend/src/pages/hardwareProfiles/const.ts b/frontend/src/pages/hardwareProfiles/const.ts index 1c40126437..ed003dfb92 100644 --- a/frontend/src/pages/hardwareProfiles/const.ts +++ b/frontend/src/pages/hardwareProfiles/const.ts @@ -4,6 +4,7 @@ import { ManageHardwareProfileSectionID, ManageHardwareProfileSectionTitlesType, } from '~/pages/hardwareProfiles/manage/types'; +import { IdentifierResourceType } from '~/types'; export const hardwareProfileColumns: SortableData[] = [ { @@ -88,6 +89,7 @@ export const DEFAULT_HARDWARE_PROFILE_SPEC: HardwareProfileKind['spec'] = { defaultCount: 2, maxCount: 4, minCount: 1, + resourceType: IdentifierResourceType.CPU, }, { identifier: 'memory', @@ -95,6 +97,7 @@ export const DEFAULT_HARDWARE_PROFILE_SPEC: HardwareProfileKind['spec'] = { defaultCount: '4Gi', minCount: '2Gi', maxCount: '8Gi', + resourceType: IdentifierResourceType.MEMORY, }, ], }; diff --git a/frontend/src/pages/hardwareProfiles/manage/ManageHardwareProfile.tsx b/frontend/src/pages/hardwareProfiles/manage/ManageHardwareProfile.tsx index 47bf8b20cc..70f3153d88 100644 --- a/frontend/src/pages/hardwareProfiles/manage/ManageHardwareProfile.tsx +++ b/frontend/src/pages/hardwareProfiles/manage/ManageHardwareProfile.tsx @@ -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 = { @@ -70,9 +69,7 @@ const ManageHardwareProfile: React.FC = ({ [state, profileNameDesc], ); - const validFormData = - isK8sNameDescriptionDataValid(profileNameDesc) && - isNodeResourcesSectionValid(state.identifiers); + const validFormData = isK8sNameDescriptionDataValid(profileNameDesc); return ( = ({ setNodeResources, }) => { const [isNodeResourceModalOpen, setIsNodeResourceModalOpen] = React.useState(false); + const isEmpty = nodeResources.length === 0; return ( <> = ({ {ManageHardwareProfileSectionTitles[ManageHardwareProfileSectionID.IDENTIFIERS]} - - - + {!isEmpty && ( + + + + )} } > - Every hardware profile must include CPU and memory resources. Additional resources, such as - GPUs, can be added here. - 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, + ) + ) && ( + + 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. + + )} + {!isEmpty && ( + setNodeResources(newResources)} + /> + )} {isNodeResourceModalOpen && ( = ({ nodeResources={nodeResources} /> )} + {isEmpty && ( + + )} ); }; diff --git a/frontend/src/pages/hardwareProfiles/nodeResource/CountFormField.tsx b/frontend/src/pages/hardwareProfiles/nodeResource/CountFormField.tsx index 9417c9b526..3fdca5f935 100644 --- a/frontend/src/pages/hardwareProfiles/nodeResource/CountFormField.tsx +++ b/frontend/src/pages/hardwareProfiles/nodeResource/CountFormField.tsx @@ -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; }; @@ -19,15 +20,15 @@ const CountFormField: React.FC = ({ fieldId, size, setSize, - identifier, + type, errorMessage, isValid = true, }) => { const renderInputField = () => { - switch (identifier) { - case 'cpu': + switch (type) { + case IdentifierResourceType.CPU: return setSize(value)} value={size} />; - case 'memory': + case IdentifierResourceType.MEMORY: return setSize(value)} value={String(size)} />; default: return ( diff --git a/frontend/src/pages/hardwareProfiles/nodeResource/ManageNodeResourceModal.tsx b/frontend/src/pages/hardwareProfiles/nodeResource/ManageNodeResourceModal.tsx index 06c1e2abd4..2d296fb59c 100644 --- a/frontend/src/pages/hardwareProfiles/nodeResource/ManageNodeResourceModal.tsx +++ b/frontend/src/pages/hardwareProfiles/nodeResource/ManageNodeResourceModal.tsx @@ -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'; @@ -32,11 +32,11 @@ const ManageNodeResourceModal: React.FC = ({ !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: @@ -74,7 +74,6 @@ const ManageNodeResourceModal: React.FC = ({ identifier={identifier} setIdentifier={setIdentifier} unitOptions={unitOptions} - isExistingIdentifier={!!existingIdentifier} isUniqueIdentifier={isUniqueIdentifier} /> diff --git a/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceForm.tsx b/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceForm.tsx index 9e572ef92f..29b600efee 100644 --- a/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceForm.tsx +++ b/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceForm.tsx @@ -7,9 +7,16 @@ import { HelperText, HelperTextItem, } from '@patternfly/react-core'; -import { Identifier } from '~/types'; +import { Identifier, IdentifierResourceType } from '~/types'; import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import { UnitOption } from '~/utilities/valueUnits'; +import SimpleSelect from '~/components/SimpleSelect'; +import { asEnumMember } from '~/utilities/utils'; +import { + DEFAULT_CPU_SIZE, + DEFAULT_MEMORY_SIZE, + EMPTY_IDENTIFIER, +} from '~/pages/hardwareProfiles/nodeResource/const'; import { validateDefaultCount, validateMinCount } from './utils'; import CountFormField from './CountFormField'; @@ -17,7 +24,6 @@ type NodeResourceFormProps = { identifier: Identifier; setIdentifier: UpdateObjectAtPropAndValue; unitOptions?: UnitOption[]; - isExistingIdentifier?: boolean; isUniqueIdentifier?: boolean; }; @@ -25,7 +31,6 @@ const NodeResourceForm: React.FC = ({ identifier, setIdentifier, unitOptions, - isExistingIdentifier, isUniqueIdentifier, }) => { const validated = isUniqueIdentifier ? 'default' : 'error'; @@ -44,10 +49,6 @@ const NodeResourceForm: React.FC = ({ setIdentifier('identifier', value)} - isDisabled={ - isExistingIdentifier && - (identifier.identifier === 'cpu' || identifier.identifier === 'memory') - } validated={validated} data-testid="node-resource-identifier-input" /> @@ -63,10 +64,47 @@ const NodeResourceForm: React.FC = ({ )} + + ({ + key: v, + label: v, + })), + { key: 'Other', label: 'Other' }, + ]} + value={identifier.resourceType || 'Other'} + onChange={(value) => { + const resourceType = asEnumMember(value, IdentifierResourceType); + switch (resourceType) { + case IdentifierResourceType.CPU: + setIdentifier('resourceType', resourceType); + setIdentifier('minCount', DEFAULT_CPU_SIZE.minCount); + setIdentifier('maxCount', DEFAULT_CPU_SIZE.maxCount); + setIdentifier('defaultCount', DEFAULT_CPU_SIZE.defaultCount); + break; + case IdentifierResourceType.MEMORY: + setIdentifier('resourceType', resourceType); + setIdentifier('minCount', DEFAULT_MEMORY_SIZE.minCount); + setIdentifier('maxCount', DEFAULT_MEMORY_SIZE.maxCount); + setIdentifier('defaultCount', DEFAULT_MEMORY_SIZE.defaultCount); + break; + default: + setIdentifier('resourceType', undefined); + setIdentifier('minCount', EMPTY_IDENTIFIER.minCount); + setIdentifier('maxCount', EMPTY_IDENTIFIER.maxCount); + setIdentifier('defaultCount', EMPTY_IDENTIFIER.defaultCount); + } + }} + /> + + setIdentifier('defaultCount', value)} isValid={validateDefaultCount(identifier, unitOptions)} @@ -76,7 +114,7 @@ const NodeResourceForm: React.FC = ({ setIdentifier('minCount', value)} isValid={validateMinCount(identifier, unitOptions)} @@ -86,7 +124,7 @@ const NodeResourceForm: React.FC = ({ setIdentifier('maxCount', value)} /> diff --git a/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceTableRow.tsx b/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceTableRow.tsx index 6d192b1129..85c296658b 100644 --- a/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceTableRow.tsx +++ b/frontend/src/pages/hardwareProfiles/nodeResource/NodeResourceTableRow.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { Td, Tr } from '@patternfly/react-table'; -import { ActionList, ActionListItem, Button, Tooltip } from '@patternfly/react-core'; +import { ActionList, ActionListItem, Button } from '@patternfly/react-core'; import { MinusCircleIcon, PencilAltIcon } from '@patternfly/react-icons'; import { Identifier } from '~/types'; @@ -16,53 +16,37 @@ const NodeResourceTableRow: React.FC = ({ onEdit, onDelete, showActions, -}) => { - const isRemoveDisabled = identifier.identifier === 'cpu' || identifier.identifier === 'memory'; - const tooltipRef = React.useRef(); - return ( - - {identifier.displayName} - {identifier.identifier} - {identifier.defaultCount} - {identifier.minCount} - {identifier.maxCount} - {showActions && ( - - - -