CNV-45584: Update Edit CPU | Memory modal UI

locales/en/plugin__kubevirt-plugin.json

@@ -17,7 +17,6 @@
   "{{ osName }} VirtualMachine can not be edited because it is provided by OpenShift Virtualization Operator.": "{{ osName }} VirtualMachine can not be edited because it is provided by OpenShift Virtualization Operator.",
   "{{annotations}} Annotations": "{{annotations}} Annotations",
   "{{annotationsCount}} Annotations": "{{annotationsCount}} Annotations",
-  "{{cores}} cores": "{{cores}} cores",
   "{{cpu}} CPU | {{memory}} Memory": "{{cpu}} CPU | {{memory}} Memory",
   "{{cpuCount}} CPU | {{memory}} Memory": "{{cpuCount}} CPU | {{memory}} Memory",
   "{{cpus}} CPUs, {{memory}} Memory": "{{cpus}} CPUs, {{memory}} Memory",
@@ -34,8 +33,6 @@
   "{{rules}} Affinity rules": "{{rules}} Affinity rules",
   "{{rules}} Tolerations rules": "{{rules}} Tolerations rules",
   "{{sizeLabel}}: {{cpus}} CPUs, {{memory}} Memory": "{{sizeLabel}}: {{cpus}} CPUs, {{memory}} Memory",
-  "{{sockets}} sockets": "{{sockets}} sockets",
-  "{{threads}} threads": "{{threads}} threads",
   "{{time}} Nanoseconds": "{{time}} Nanoseconds",
   "{{time}} seconds": "{{time}} seconds",
   "{{timestampPluralized}} ago": "{{timestampPluralized}} ago",
@@ -154,6 +151,7 @@
   "Are you sure you want to cancel?": "Are you sure you want to cancel?",
   "Are you sure you want to leave this page?": "Are you sure you want to leave this page?",
   "Are you sure you want to restore {{vmName}} from snapshot {{snapshotName}}?<6><0>Note: </0>Data from the last snapshot taken will be lost. To prevent losing current data, take another snapshot before restoring from this one.</6>": "Are you sure you want to restore {{vmName}} from snapshot {{snapshotName}}?<6><0>Note: </0>Data from the last snapshot taken will be lost. To prevent losing current data, take another snapshot before restoring from this one.</6>",
+  "As a default, the VirtualMachine CPU uses sockets to enable hotplug. You can also define the topology manually": "As a default, the VirtualMachine CPU uses sockets to enable hotplug. You can also define the topology manually",
   "As new versions of a DataSource become available older versions will be replaced": "As new versions of a DataSource become available older versions will be replaced",
   "Ask your cluster administrator for access permissions.": "Ask your cluster administrator for access permissions.",
   "Assigns an external IP address to the VirtualMachine. This option requires a LoadBalancer Service backend": "Assigns an external IP address to the VirtualMachine. This option requires a LoadBalancer Service backend",
@@ -309,14 +307,13 @@
   "Copy template's boot source disk": "Copy template's boot source disk",
   "Copy to clipboard": "Copy to clipboard",
   "Copying files": "Copying files",
+  "Cores": "Cores",
   "Could not create persistent volume claim": "Could not create persistent volume claim",
   "CPU": "CPU",
   "CPU | Memory": "CPU | Memory",
   "CPU and Memory can not be edited if the VirtualMachine is created from InstanceType": "CPU and Memory can not be edited if the VirtualMachine is created from InstanceType",
-  "CPU sockets": "CPU sockets",
   "CPU used": "CPU used",
   "CPUs": "CPUs",
-  "CPUs = sockets x threads x cores": "CPUs = sockets x threads x cores",
   "CPUs = sockets x threads x cores.": "CPUs = sockets x threads x cores.",
   "Create": "Create",
   "Create a copy of the VirtualMachine from snapshot": "Create a copy of the VirtualMachine from snapshot",
@@ -539,6 +536,7 @@
   "Filesystem": "Filesystem",
   "Filter": "Filter",
   "Filter by keyword...": "Filter by keyword...",
+  "Final topology is {{sockets}} socket, 1 core, 1 thread": "Final topology is {{sockets}} socket, 1 core, 1 thread",
   "Flavor": "Flavor",
   "Follow guided documentation to build applications and familiarize yourself with key features.": "Follow guided documentation to build applications and familiarize yourself with key features.",
   "Force stop": "Force stop",
@@ -587,6 +585,7 @@
   "Hot plug is enabled only for \"Bridge\" and \"SR-IOV\" types": "Hot plug is enabled only for \"Bridge\" and \"SR-IOV\" types",
   "Hot plug is enabled only for \"Disk\" and \"Lun\" types": "Hot plug is enabled only for \"Disk\" and \"Lun\" types",
   "Hot plug is enabled only for \"SCSI\" interface": "Hot plug is enabled only for \"SCSI\" interface",
+  "Hot-plug is only possible with Sockets. Cores/Threads editing requires restarting the VirtualMachine": "Hot-plug is only possible with Sockets. Cores/Threads editing requires restarting the VirtualMachine",
   "How much time before the check will try to close itself": "How much time before the check will try to close itself",
   "HP series": "HP series",
   "If no nodes are specified, random nodes are selected.": "If no nodes are specified, random nodes are selected.",
@@ -1087,6 +1086,7 @@
   "Service Accounts": "Service Accounts",
   "Services": "Services",
   "Services ({{services}})": "Services ({{services}})",
+  "Set CPU topology manually": "Set CPU topology manually",
   "Set live migration limits": "Set live migration limits",
   "Set live migration network": "Set live migration network",
   "Set maximum desired latency (milliseconds)": "Set maximum desired latency (milliseconds)",
@@ -1112,6 +1112,7 @@
   "Small scale consumption, recommended for using the graphical console": "Small scale consumption, recommended for using the graphical console",
   "Snapshots": "Snapshots",
   "Snapshots ({{snapshots}})": "Snapshots ({{snapshots}})",
+  "Sockets": "Sockets",
   "Some fields may not be supported.": "Some fields may not be supported.",
   "Source": "Source",
   "Source available": "Source available",
@@ -1242,6 +1243,7 @@
   "This VirtualMachine is down. Please start it to access its console.": "This VirtualMachine is down. Please start it to access its console.",
   "This VirtualMachine is subject to the Descheduler profiles configured for eviction.": "This VirtualMachine is subject to the Descheduler profiles configured for eviction.",
   "This will create a cloned copy of the PVC in the destination project.": "This will create a cloned copy of the PVC in the destination project.",
+  "Threads": "Threads",
   "Time axis": "Time axis",
   "Time is a wrapper around time.Time which supports correct marshaling to YAML and JSON.  Wrappers are provided for many of the factory methods that the time package offers.": "Time is a wrapper around time.Time which supports correct marshaling to YAML and JSON.  Wrappers are provided for many of the factory methods that the time package offers.",
   "Time of login": "Time of login",

src/utils/components/CPUMemoryModal/CPUMemoryModal.tsx

@@ -1,16 +1,22 @@
-import React, { FC, useEffect, useMemo, useState } from 'react';
+import React, { FC, useState } from 'react';
 import produce from 'immer';
 
-import { V1VirtualMachine } from '@kubevirt-ui/kubevirt-api/kubevirt';
+import { V1CPU, V1VirtualMachine } from '@kubevirt-ui/kubevirt-api/kubevirt';
 import CPUInput from '@kubevirt-utils/components/CPUMemoryModal/components/CPUInput/CPUInput';
-import CPUMemoryModalHeader from '@kubevirt-utils/components/CPUMemoryModal/components/CPUInput/CPUMemoryModalHeader';
 import MemoryInput from '@kubevirt-utils/components/CPUMemoryModal/components/MemoryInput/MemoryInput';
 import { DEFAULT_NAMESPACE } from '@kubevirt-utils/constants/constants';
 import { useKubevirtTranslation } from '@kubevirt-utils/hooks/useKubevirtTranslation';
 import { getLabel } from '@kubevirt-utils/resources/shared';
-import { getCPUSockets, getMemory, VM_TEMPLATE_ANNOTATION } from '@kubevirt-utils/resources/vm';
+import { getCPU, getMemory, VM_TEMPLATE_ANNOTATION } from '@kubevirt-utils/resources/vm';
 import { ensurePath } from '@kubevirt-utils/utils/utils';
-import { Alert, Button, ButtonVariant, Modal, ModalVariant } from '@patternfly/react-core';
+import {
+  Alert,
+  AlertVariant,
+  Button,
+  ButtonVariant,
+  Modal,
+  ModalVariant,
+} from '@patternfly/react-core';
 
 import useTemplateDefaultCpuMemory from './hooks/useTemplateDefaultCpuMemory';
 import { getMemorySize } from './utils/CpuMemoryUtils';
@@ -46,41 +52,29 @@ const CPUMemoryModal: FC<CPUMemoryModalProps> = ({
   const [updateInProcess, setUpdateInProcess] = useState<boolean>(false);
   const [updateError, setUpdateError] = useState<string>();
 
-  const [memory, setMemory] = useState<number>();
-  const [cpuSockets, setCPUSockets] = useState<number>();
-  const [memoryUnit, setMemoryUnit] = useState<string>();
+  const { size, unit } = getMemorySize(getMemory(vm));
+  const [memory, setMemory] = useState<number>(size || undefined);
+  const [memoryUnit, setMemoryUnit] = useState<string>(unit || undefined);
+  const [cpu, setCPU] = useState<V1CPU>(getCPU(vm));
 
   const templateName = getLabel(vm, VM_TEMPLATE_ANNOTATION);
 
-  const updatedVirtualMachine = useMemo(() => {
+  const handleSubmit = async () => {
+    setUpdateInProcess(true);
+    setUpdateError(null);
+
     const updatedVM = produce<V1VirtualMachine>(vm, (vmDraft: V1VirtualMachine) => {
       ensurePath(vmDraft, [
         'spec.template.spec.domain.cpu',
         'spec.template.spec.domain.memory.guest',
       ]);
 
-      vmDraft.spec.template.spec.domain.cpu.sockets = cpuSockets;
+      vmDraft.spec.template.spec.domain.cpu = cpu;
       vmDraft.spec.template.spec.domain.memory.guest = `${memory}${memoryUnit}`;
     });
 
-    return updatedVM;
-  }, [vm, memory, cpuSockets, memoryUnit]);
-
-  useEffect(() => {
-    if (vm?.metadata) {
-      const { size, unit } = getMemorySize(getMemory(vm));
-      setMemoryUnit(unit);
-      setMemory(size);
-      setCPUSockets(getCPUSockets(vm));
-    }
-  }, [vm]);
-
-  const handleSubmit = async () => {
-    setUpdateInProcess(true);
-    setUpdateError(null);
-
     try {
-      await onSubmit(updatedVirtualMachine);
+      await onSubmit(updatedVM);
 
       setUpdateInProcess(false);
       onClose();
@@ -111,7 +105,7 @@ const CPUMemoryModal: FC<CPUMemoryModalProps> = ({
             defaultLoadError
           }
           onClick={() => {
-            setCPUSockets(templateDefaultsData?.defaultCpu);
+            setCPU(templateDefaultsData?.defaultCpu);
             setMemory(templateDefaultsData?.defaultMemory?.size);
             setMemoryUnit(templateDefaultsData?.defaultMemory?.unit);
           }}
@@ -126,13 +120,20 @@ const CPUMemoryModal: FC<CPUMemoryModalProps> = ({
         </Button>,
       ]}
       className="cpu-memory-modal"
-      header={<CPUMemoryModalHeader />}
       isOpen={isOpen}
       onClose={onClose}
+      title={t('Edit CPU | Memory')}
       variant={ModalVariant.small}
+      width="650px"
     >
+      <Alert
+        title={t(
+          'Hot-plug is only possible with Sockets. Cores/Threads editing requires restarting the VirtualMachine',
+        )}
+        variant={AlertVariant.info}
+      />
       <div className="inputs">
-        <CPUInput cpuSockets={cpuSockets} setCPUSockets={setCPUSockets} vm={vm} />
+        <CPUInput setCPU={setCPU} vm={vm} />
         <MemoryInput
           memory={memory}
           memoryUnit={memoryUnit}

src/utils/components/CPUMemoryModal/components/CPUInput/CPUInput.scss

@@ -1,12 +1,19 @@
 .cpu-input {
   margin-right: var(--pf-global--spacer--xl);
 
-  &__help-text-button {
-    padding-top: 0;
-    padding-left: var(--pf-global--spacer--xs);
+  &__edit-topology-manually {
+    margin-top: var(--pf-global--spacer--xl);
   }
 
   &__number-input {
     padding-bottom: var(--pf-global--spacer--xs);
   }
+
+  &__title {
+    margin-bottom: var(--pf-global--spacer--md);
+  }
+
+  &__title--help-text-button {
+    padding: 0 var(--pf-global--spacer--xs);
+  }
 }

src/utils/components/CPUMemoryModal/components/CPUInput/CPUInput.tsx

@@ -1,39 +1,97 @@
-import React, { ChangeEvent, Dispatch, FC, SetStateAction } from 'react';
+import React, { Dispatch, FC, SetStateAction, useState } from 'react';
 
-import { V1VirtualMachine } from '@kubevirt-ui/kubevirt-api/kubevirt';
-import CPUHelperText from '@kubevirt-utils/components/CPUMemoryModal/components/CPUInput/CPUHelperText';
+import { V1CPU, V1VirtualMachine } from '@kubevirt-ui/kubevirt-api/kubevirt';
+import VCPUInput from '@kubevirt-utils/components/CPUMemoryModal/components/CPUInput/components/vCPUInput/VCPUInput';
+import {
+  convertTopologyToVCPUs,
+  CPUInputType,
+  formatVCPUsAsSockets,
+} from '@kubevirt-utils/components/CPUMemoryModal/components/CPUInput/utils/utils';
 import { useKubevirtTranslation } from '@kubevirt-utils/hooks/useKubevirtTranslation';
 import { getCPU } from '@kubevirt-utils/resources/vm';
-import { NumberInput, Title, TitleSizes } from '@patternfly/react-core';
+import { Button, ButtonVariant, Popover, Radio, Title, TitleSizes } from '@patternfly/react-core';
+import { HelpIcon } from '@patternfly/react-icons';
+
+import CPUTopologyInput from './components/CPUTopologyInput/CPUTopologyInput';
+import CPUHelperText from './components/vCPUInput/components/CPUHelperText/CPUHelperText';
 
 import './CPUInput.scss';
 
 type CPUInputProps = {
-  cpuSockets: number;
-  setCPUSockets: Dispatch<SetStateAction<number>>;
+  setCPU: Dispatch<SetStateAction<V1CPU>>;
   vm: V1VirtualMachine;
 };
 
-const CPUInput: FC<CPUInputProps> = ({ cpuSockets, setCPUSockets, vm }) => {
+const CPUInput: FC<CPUInputProps> = ({ setCPU, vm }) => {
   const { t } = useKubevirtTranslation();
+  const [selectedRadioOption, setSelectedRadioOption] = useState<CPUInputType>(
+    CPUInputType.editVCPU,
+  );
+
+  const [socketsEditedVCPU, setSocketsEditedVCPU] = useState<V1CPU>(
+    formatVCPUsAsSockets(getCPU(vm)),
+  );
+  const [topologyEditedVCPU, setTopologyEditedVCPU] = useState<V1CPU>(getCPU(vm));
+
+  const radioInputName = 'cpu-input-type';
 
   return (
-    <div className="input-cpu">
-      <Title headingLevel="h6" size={TitleSizes.md}>
-        {t('CPU sockets')}
+    <div className="cpu-input">
+      <Title className="cpu-input__title" headingLevel="h6" size={TitleSizes.md}>
+        {t('CPU')}
+        <Popover
+          bodyContent={t(
+            'As a default, the VirtualMachine CPU uses sockets to enable hotplug. You can also define the topology manually',
+          )}
+        >
+          <Button
+            aria-label="Action"
+            className="cpu-input__title--help-text-button"
+            variant={ButtonVariant.plain}
+          >
+            <HelpIcon />
+          </Button>
+        </Popover>
       </Title>
-      <NumberInput
-        onChange={(e: ChangeEvent<HTMLInputElement>) => {
-          const newNumber = +e?.target?.value;
-          setCPUSockets((cpus) => (newNumber > 0 ? newNumber : cpus));
+      <Radio
+        body={
+          <VCPUInput
+            cpu={socketsEditedVCPU}
+            setCPU={setSocketsEditedVCPU}
+            setSelectedRadioOption={setSelectedRadioOption}
+          />
+        }
+        onClick={() => {
+          setSelectedRadioOption(CPUInputType.editVCPU);
+          setCPU(socketsEditedVCPU);
+        }}
+        id={CPUInputType.editVCPU}
+        isChecked={selectedRadioOption === CPUInputType.editVCPU}
+        isLabelWrapped
+        name={radioInputName}
+      />
+      <CPUHelperText
+        hide={socketsEditedVCPU?.sockets === convertTopologyToVCPUs(getCPU(vm))}
+        sockets={socketsEditedVCPU?.sockets}
+      />
+      <Radio
+        body={
+          <CPUTopologyInput
+            cpu={topologyEditedVCPU}
+            hide={selectedRadioOption !== CPUInputType.editTopologyManually}
+            setCPU={setTopologyEditedVCPU}
+          />
+        }
+        onClick={() => {
+          setSelectedRadioOption(CPUInputType.editTopologyManually);
+          setCPU(topologyEditedVCPU);
         }}
-        inputName="cpu-input"
-        min={1}
-        onMinus={() => setCPUSockets((cpus) => +cpus - 1)}
-        onPlus={() => setCPUSockets((cpus) => +cpus + 1)}
-        value={cpuSockets}
+        className="cpu-input__edit-topology-manually"
+        id={CPUInputType.editTopologyManually}
+        isChecked={selectedRadioOption === CPUInputType.editTopologyManually}
+        label={<>{t('Set CPU topology manually')}</>}
+        name={radioInputName}
       />
-      <CPUHelperText cpu={getCPU(vm)} sockets={cpuSockets} />
     </div>
   );
 };