Skip to content
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
5 changes: 5 additions & 0 deletions frontend/packages/console-shared/src/selectors/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export const getLabels = <A extends K8sResourceCommon = K8sResourceCommon>(
value: A,
defaultValue?: K8sResourceCommon['metadata']['labels'],
) => (_.has(value, 'metadata.labels') ? value.metadata.labels : defaultValue);
export const getLabel = <A extends K8sResourceCommon = K8sResourceCommon>(
value: A,
label: string,
defaultValue?: string,
) => (_.has(value, 'metadata.labels') ? value.metadata.labels[label] : defaultValue);
export const getAnnotations = <A extends K8sResourceCommon = K8sResourceCommon>(
value: A,
defaultValue?: K8sResourceCommon['metadata']['annotations'],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const DEDICATED_RESOURCES = 'Dedicated Resources';
export const RESOURCE_PINNED = 'Workload scheduled with dedicated resources (guaranteed policy)';
export const RESOURCE_NOT_PINNED = 'No Dedicated resources applied';
export const RESOURCE_NO_NODES_AVAILABLE = 'There are no qualified Nodes with the required labels';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.kubevirt-cpu-pinning__checkbox {
margin-top: var(--pf-global--spacer--md);
margin-bottom: var(--pf-global--spacer--md);
}

.kubevirt-cpu-pinning__helper-text {
color: var(--pf-global--Color--200);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import * as React from 'react';
import { Checkbox, Modal, Text, TextVariants } from '@patternfly/react-core';
import { k8sPatch } from '@console/internal/module/k8s';
import { NodeModel } from '@console/internal/models';
import { getLabel } from '@console/shared';
import {
Firehose,
withHandlePromise,
HandlePromiseProps,
FirehoseResult,
Label,
} from '@console/internal/components/utils';
import { ModalFooter } from '../modal/modal-footer';
import { getVMLikeModel, isDedicatedCPUPlacement, asVM } from '../../../selectors/vm';
import { getDedicatedCpuPatch } from '../../../k8s/patches/vm/vm-cpu-patches';
import { VMLikeEntityKind } from '../../../types';
import { getLoadedData, isLoaded, getLoadError } from '../../../utils';
import { RESOURCE_NO_NODES_AVAILABLE, DEDICATED_RESOURCES } from './consts';
import './dedicated-resources-modal.scss';

const ResourceModal = withHandlePromise<ResourceModalProps>(
({ vmLikeEntity, nodes, isOpen, setOpen, handlePromise, inProgress, errorMessage }) => {
const isLoading = !isLoaded(nodes);
const loadError = getLoadError(nodes, NodeModel);
const isCPUPinned = isDedicatedCPUPlacement(asVM(vmLikeEntity));
const loadedNodes = getLoadedData(nodes, []);

const [isPinned, setIsPinned] = React.useState<boolean>(isCPUPinned);
const [showPatchError, setPatchError] = React.useState<boolean>(false);
const isNodeAvailable = loadedNodes.some((node) => getLabel(node, 'cpumanager') === 'true');

const submit = async () => {
if (isPinned !== isCPUPinned) {
handlePromise(
k8sPatch(
getVMLikeModel(vmLikeEntity),
vmLikeEntity,
await getDedicatedCpuPatch(vmLikeEntity, isPinned),
),
)
.then(() => setOpen(false))
.catch(() => setPatchError(true));
} else {
setOpen(false);
}
};
const footer = (
<ModalFooter
className=""
warningMessage={!loadError && !isNodeAvailable && RESOURCE_NO_NODES_AVAILABLE}
errorMessage={showPatchError && errorMessage}
inProgress={inProgress || isLoading}
isSimpleError={!isNodeAvailable}
onSubmit={submit}
onCancel={() => setOpen(false)}
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that its better to pass onClose which wont take any parameter as input as you always call setOpen(false).

Copy link
Contributor

Choose a reason for hiding this comment

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

actually pass the name should be close as defined in ModalComponentProps

submitButtonText="Save"
/>
);

return (
<Modal
width="50%"
title={DEDICATED_RESOURCES}
isOpen={isOpen}
onClose={() => setOpen(false)}
footer={footer}
isFooterLeftAligned
>
<Checkbox
className="kubevirt-cpu-pinning__checkbox"
label="Schedule this workload with dedicated resources (guaranteed policy)"
isChecked={isPinned}
isDisabled={isLoading}
onChange={setIsPinned}
id="cpu-pinning-checkbox"
/>
<Text className="kubevirt-cpu-pinning__helper-text" component={TextVariants.small}>
Available only on Nodes with labels{' '}
<Label kind={NodeModel.kind} name="cpumanager" value="true" expand />
</Text>
</Modal>
);
},
);

type ResourceModalProps = HandlePromiseProps & {
vmLikeEntity: VMLikeEntityKind;
isOpen: boolean;
nodes?: FirehoseResult<VMLikeEntityKind[]>;
setOpen: (isOpen: boolean) => void;
};

export const DedicatedResourcesModal: React.FC<DedicatedResourcesModalProps> = (props) => {
const { vmLikeEntity, ...restProps } = props;

const resources = [
{
kind: NodeModel.kind,
isList: true,
namespaced: false,
prop: 'nodes',
},
];

return (
<Firehose resources={resources}>
<ResourceModal vmLikeEntity={vmLikeEntity} {...restProps} />
</Firehose>
);
};

type DedicatedResourcesModalProps = {
vmLikeEntity: VMLikeEntityKind;
isOpen: boolean;
setOpen: (isOpen: boolean) => void;
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.kubevirt-create-nic-modal__buttons {
padding: 0;
text-align: left;
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import { Alert, Button, ButtonVariant } from '@patternfly/react-core';
import classNames from 'classnames';
import { Alert, Button, ButtonVariant, AlertProps } from '@patternfly/react-core';
import { LoadingInline } from '@console/internal/components/utils';
import { prefixedID } from '../../../utils';

Expand All @@ -20,13 +21,15 @@ export const ModalErrorMessage: React.FC<ModalErrorMessageProps> = ({ message })
</Alert>
);

type ModalSimpleErrorMessageProps = {
type ModalSimpleMessageProps = {
message: string;
variant?: AlertProps['variant'];
};

export const ModalSimpleErrorMessage: React.FC<ModalSimpleErrorMessageProps> = ({ message }) => (
<Alert isInline className="co-alert" variant="danger" title={message} />
);
export const ModalSimpleMessage: React.FC<ModalSimpleMessageProps> = ({
message,
variant = 'danger',
}) => <Alert isInline className="co-alert" variant={variant} title={message} />;

type ModalInfoMessageProps = {
title: string;
Expand All @@ -41,7 +44,9 @@ export const ModalInfoMessage: React.FC<ModalInfoMessageProps> = ({ title, child

type ModalFooterProps = {
id?: string;
className?: string;
errorMessage?: string;
warningMessage?: string;
isSimpleError?: boolean;
onSubmit: (e) => void;
onCancel: (e) => void;
Expand All @@ -55,7 +60,9 @@ type ModalFooterProps = {

export const ModalFooter: React.FC<ModalFooterProps> = ({
id,
className = '',
errorMessage = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

why all the null and false values as defaults ? you can keep the these props empty and the result will be the same and the function declaration more tidy

warningMessage = null,
isDisabled = false,
inProgress = false,
isSimpleError = false,
Expand All @@ -66,8 +73,16 @@ export const ModalFooter: React.FC<ModalFooterProps> = ({
infoMessage = null,
infoTitle = null,
}) => (
<footer className="co-m-btn-bar modal-footer kubevirt-create-nic-modal__buttons">
{errorMessage && isSimpleError && <ModalSimpleErrorMessage message={errorMessage} />}
<footer
className={classNames(
'co-m-btn-bar modal-footer kubevirt-create-nic-modal__buttons',
className,
)}
>
{warningMessage && isSimpleError && (
<ModalSimpleMessage message={warningMessage} variant="warning" />
)}
{errorMessage && isSimpleError && <ModalSimpleMessage message={errorMessage} />}
{errorMessage && !isSimpleError && <ModalErrorMessage message={errorMessage} />}
{infoTitle && <ModalInfoMessage title={infoTitle}>{infoMessage}</ModalInfoMessage>}
<Button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { getBasicID, prefixedID } from '../../utils';
import { vmDescriptionModal } from '../modals/vm-description-modal';
import { BootOrderModal } from '../modals/boot-order-modal';
import { VMCDRomModal } from '../modals/cdrom-vm-modal';
import { DedicatedResourcesModal } from '../modals/dedicated-resources-modal/dedicated-resources-modal';
import { getDescription } from '../../selectors/selectors';
import {
getCDRoms,
getOperatingSystemName,
getOperatingSystem,
getWorkloadProfile,
isDedicatedCPUPlacement,
} from '../../selectors/vm/selectors';
import { getVMTemplateNamespacedName } from '../../selectors/vm-template/selectors';
import { vmFlavorModal } from '../modals';
Expand All @@ -22,6 +24,11 @@ import { DiskSummary } from '../vm-disks/disk-summary';
import { asVM, getDevices } from '../../selectors/vm';
import { BootOrderSummary } from '../boot-order';
import { V1alpha1DataVolume } from '../../types/vm/disk/V1alpha1DataVolume';
import {
RESOURCE_PINNED,
RESOURCE_NOT_PINNED,
DEDICATED_RESOURCES,
} from '../modals/dedicated-resources-modal/consts';
import { VMTemplateLink } from './vm-template-link';
import { TemplateSource } from './vm-template-source';

Expand Down Expand Up @@ -83,11 +90,15 @@ export const VMTemplateDetailsList: React.FC<VMTemplateResourceListProps> = ({
canUpdateTemplate,
}) => {
const [isBootOrderModalOpen, setBootOrderModalOpen] = React.useState<boolean>(false);
const [isDedicatedResourcesModalOpen, setDedicatedResourcesModalOpen] = React.useState<boolean>(
false,
);

const id = getBasicID(template);
const devices = getDevices(template);
const cds = getCDRoms(asVM(template));
const flavorText = getFlavorText(template);
const isCPUPinned = isDedicatedCPUPlacement(asVM(template));

return (
<dl className="co-m-pane__details">
Expand Down Expand Up @@ -127,6 +138,21 @@ export const VMTemplateDetailsList: React.FC<VMTemplateResourceListProps> = ({
</EditButton>
</VMDetailsItem>

<VMDetailsItem
title={DEDICATED_RESOURCES}
idValue={prefixedID(id, 'dedicated-resources')}
canEdit
onEditClick={() => setDedicatedResourcesModalOpen(true)}
editButtonId={prefixedID(id, 'dedicated-resources-edit')}
>
<DedicatedResourcesModal
vmLikeEntity={template}
isOpen={isDedicatedResourcesModalOpen}
setOpen={setDedicatedResourcesModalOpen}
Copy link
Contributor

@rawagner rawagner Jan 21, 2020

Choose a reason for hiding this comment

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

as metioned above, it would be better to pass onClose callback

const onClose = React.useCallback(() => setDedicatedResourcesModalOpen(false), []);
...
<DedicatedResourcesModal
          vmLikeEntity={template}
          isOpen={isDedicatedResourcesModalOpen}
          onClose={onClose}



/>
{isCPUPinned ? RESOURCE_PINNED : RESOURCE_NOT_PINNED}
</VMDetailsItem>

<VMDetailsItem title="Provision Source" idValue={prefixedID(id, 'provisioning-source')}>
<TemplateSource template={template} dataVolumeLookup={dataVolumeLookup} detailed />
</VMDetailsItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import { VMTemplateLink } from '../vm-templates/vm-template-link';
import { getBasicID, prefixedID } from '../../utils';
import { vmDescriptionModal, vmFlavorModal } from '../modals';
import { VMCDRomModal } from '../modals/cdrom-vm-modal';
import { DedicatedResourcesModal } from '../modals/dedicated-resources-modal/dedicated-resources-modal';
import { BootOrderModal } from '../modals/boot-order-modal/boot-order-modal';
import { getDescription } from '../../selectors/selectors';
import { getCDRoms } from '../../selectors/vm/selectors';
import { getCDRoms, isDedicatedCPUPlacement } from '../../selectors/vm/selectors';
import { getVMTemplateNamespacedName } from '../../selectors/vm-template/selectors';
import { getVMStatus } from '../../statuses/vm/vm';
import { getFlavorText } from '../flavor-text';
Expand All @@ -20,6 +21,11 @@ import { VMStatuses } from '../vm-status';
import { DiskSummary } from '../vm-disks/disk-summary';
import { BootOrderSummary } from '../boot-order';
import { VirtualMachineInstanceModel } from '../../models';
import {
RESOURCE_PINNED,
RESOURCE_NOT_PINNED,
DEDICATED_RESOURCES,
} from '../modals/dedicated-resources-modal/consts';
import {
getOperatingSystemName,
getOperatingSystem,
Expand Down Expand Up @@ -94,6 +100,9 @@ export const VMDetailsList: React.FC<VMResourceListProps> = ({
canUpdateVM,
}) => {
const [isBootOrderModalOpen, setBootOrderModalOpen] = React.useState<boolean>(false);
const [isDedicatedResourcesModalOpen, setDedicatedResourcesModalOpen] = React.useState<boolean>(
false,
);

const id = getBasicID(vm);
const vmStatus = getVMStatus({ vm, vmi, pods, migrations });
Expand All @@ -104,6 +113,7 @@ export const VMDetailsList: React.FC<VMResourceListProps> = ({
const ipAddrs = getVmiIpAddressesString(vmi, vmStatus);
const workloadProfile = getWorkloadProfile(vm);
const flavorText = getFlavorText(vm);
const isCPUPinned = isDedicatedCPUPlacement(vm);

return (
<dl className="co-m-pane__details">
Expand Down Expand Up @@ -177,6 +187,21 @@ export const VMDetailsList: React.FC<VMResourceListProps> = ({
</EditButton>
</VMDetailsItem>

<VMDetailsItem
title={DEDICATED_RESOURCES}
idValue={prefixedID(id, 'dedicated-resources')}
canEdit
onEditClick={() => setDedicatedResourcesModalOpen(true)}
editButtonId={prefixedID(id, 'dedicated-resources-edit')}
>
<DedicatedResourcesModal
vmLikeEntity={vm}
isOpen={isDedicatedResourcesModalOpen}
setOpen={setDedicatedResourcesModalOpen}
/>
{isCPUPinned ? RESOURCE_PINNED : RESOURCE_NOT_PINNED}
</VMDetailsItem>

<VMDetailsItem
title="Workload Profile"
idValue={prefixedID(id, 'workload-profile')}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { PatchBuilder, PatchOperation } from '@console/shared/src/k8s';
import { Patch } from '@console/internal/module/k8s';
import { VMLikeEntityKind } from '../../../types';
import { getVMLikePatches } from '../vm-template';
import {
getCPU,
getResourcesRequestsCPUCount,
getResourcesLimitsCPUCount,
asVM,
} from '../../../selectors/vm';

export const getDedicatedCpuPatch = (
vmLikeEntity: VMLikeEntityKind,
dedicatedCpuPlacement: boolean,
): Patch[] => {
const vm = asVM(vmLikeEntity);
const isCPUAvailable = !!getCPU(vm);
const patches = [];

if (isCPUAvailable) {
patches.push(
new PatchBuilder('/spec/template/spec/domain/cpu/dedicatedCpuPlacement')
.setOperation(PatchOperation.REPLACE)
.setValue(dedicatedCpuPlacement)
.build(),
);
} else {
const resourcesCPU = getResourcesRequestsCPUCount(vm) || getResourcesLimitsCPUCount(vm);
patches.push(
new PatchBuilder('/spec/template/spec/domain/cpu')
.setOperation(PatchOperation.REPLACE)
.setValue(resourcesCPU ? { dedicatedCpuPlacement } : { cores: 1, dedicatedCpuPlacement })
.build(),
);
}

return getVMLikePatches(vmLikeEntity, () => patches);
Copy link
Member

Choose a reason for hiding this comment

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

vm is passed in the (vm) => patches function so you don't have to call asVM in the beggining

Copy link
Contributor Author

Choose a reason for hiding this comment

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

asVM is for the getCPU

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm going one level deeper, patching just dedicatedCpuPlacement so no need for cpu parsing

Copy link
Member

Choose a reason for hiding this comment

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

this will not work if the cpu field is missing

};
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ import { vCPUCount } from './cpu';
export const getMemory = (vm: VMKind) =>
_.get(vm, 'spec.template.spec.domain.resources.requests.memory');
export const getCPU = (vm: VMKind): CPURaw => _.get(vm, 'spec.template.spec.domain.cpu');
export const getResourcesRequestsCPUCount = (vm: VMKind): string =>
vm?.spec?.template?.spec?.domain?.resources?.requests?.cpu;
export const getResourcesLimitsCPUCount = (vm: VMKind): string =>
vm?.spec?.template?.spec?.domain?.resources?.limits?.cpu;
export const isDedicatedCPUPlacement = (vm: VMKind) =>
_.get(vm, 'spec.template.spec.domain.cpu.dedicatedCpuPlacement');
export const getDisks = (vm: VMKind, defaultValue = []): V1Disk[] =>
_.get(vm, 'spec.template.spec.domain.devices.disks') == null
? defaultValue
Expand Down
Loading