Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Display error on duplicate VM name in wizard #449

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
12 changes: 8 additions & 4 deletions src/components/Wizard/CreateVmWizard/CreateVmWizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const getUpdatedValidatedState = (prevProps, prevState, props, state, extra) =>

const validatedStepData = ALL_TAB_KEYS.reduce((resultStepData, tabKey) => {
const { value, valid } = newState.stepData[tabKey];
resultStepData[tabKey] = validateTabData(tabKey, value, valid);
resultStepData[tabKey] = validateTabData(tabKey, value, valid, props);
return resultStepData;
}, {});

Expand Down Expand Up @@ -84,6 +84,7 @@ export class CreateVmWizard extends React.Component {
this.state = getUpdatedValidatedState(null, null, props, initialState, {
// eslint-disable-next-line no-console
safeSetState: () => console.warn('setState not supported when initializing'),
virtualMachines: this.props.virtualMachines,
});
}

Expand All @@ -104,6 +105,7 @@ export class CreateVmWizard extends React.Component {
componentDidUpdate(prevProps, prevState, snapshot) {
const newState = getUpdatedValidatedState(prevProps, prevState, this.props, this.state, {
safeSetState: this.safeSetState,
virtualMachines: this.props.virtualMachines,
});

if (this.state !== newState) {
Expand All @@ -117,7 +119,7 @@ export class CreateVmWizard extends React.Component {
lastStepReached = () => this.state.activeStepIndex === this.getLastStepIndex();

onStepDataChanged = (tabKey, value, valid, lockStep = false) => {
const validatedTabData = validateTabData(tabKey, value, valid);
const validatedTabData = validateTabData(tabKey, value, valid, this.props);
this.safeSetState(state => ({
stepData: {
...state.stepData,
Expand Down Expand Up @@ -185,9 +187,9 @@ export class CreateVmWizard extends React.Component {
key: VM_SETTINGS_TAB_KEY,
onCloseWizard: onCloseVmSettings,
render: () => {
const { namespaces, templates, dataVolumes, WithResources } = this.props;
const { namespaces, templates, dataVolumes, virtualMachines, WithResources } = this.props;

const loadingData = { namespaces, templates, dataVolumes };
const loadingData = { namespaces, templates, dataVolumes, virtualMachines };
const vmSettings = getVmSettings(this.state);

return (
Expand Down Expand Up @@ -305,6 +307,7 @@ export class CreateVmWizard extends React.Component {
CreateVmWizard.defaultProps = {
templates: null,
namespaces: null,
virtualMachines: null,
selectedNamespace: null,
networkConfigs: null,
persistentVolumeClaims: null,
Expand All @@ -322,6 +325,7 @@ CreateVmWizard.propTypes = {
onHide: PropTypes.func.isRequired,
templates: PropTypes.array,
namespaces: PropTypes.array,
virtualMachines: PropTypes.array,
selectedNamespace: PropTypes.object,
networkConfigs: PropTypes.array,
persistentVolumeClaims: PropTypes.array,
Expand Down
3 changes: 3 additions & 0 deletions src/components/Wizard/CreateVmWizard/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,6 @@ export const STORAGE_TYPE_CONTAINER = 'container';
export const DATA_VOLUME_SOURCE_BLANK = 'datavolume-blank';
export const DATA_VOLUME_SOURCE_URL = 'datavolume-url';
export const DATA_VOLUME_SOURCE_PVC = 'datavolume-pvc';

// Additional resource keys
export const VIRTUAL_MACHINES_KEY = 'virtualMachines';
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export default [
storageClasses,
units,
dataVolumes: [urlTemplateDataVolume],
virtualMachines: [],
},
},
{
Expand All @@ -80,6 +81,7 @@ export default [
storageClasses,
units,
dataVolumes: [urlTemplateDataVolume],
virtualMachines: [],
},
},
{
Expand All @@ -95,6 +97,7 @@ export default [
storageClasses: null,
units,
dataVolumes: null,
virtualMachines: null,
},
},
{
Expand All @@ -111,6 +114,7 @@ export default [
units,
createTemplate: true,
dataVolumes: [urlTemplateDataVolume],
virtualMachines: [],
},
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ const testWalkThrough = (template = false, createText = CREATE_VM, templatesDrop
const namespaces = [...CreateVmWizardFixutre[0].props.namespaces];
const templates = [...CreateVmWizardFixutre[0].props.templates];
const dataVolumes = [...CreateVmWizardFixutre[0].props.dataVolumes];
component.setProps({ namespaces, templates, dataVolumes });
const virtualMachines = [];
component.setProps({ namespaces, templates, dataVolumes, virtualMachines });

expect(component.find(Loading)).toHaveLength(0);
expect(component.find(VmSettingsTab)).toHaveLength(1);
Expand Down
4 changes: 2 additions & 2 deletions src/components/Wizard/CreateVmWizard/validations/utils.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { get } from 'lodash';

export const asGenericFieldValidator = (customValidator, getFieldTitle) => (key, vmSettings) => {
export const asGenericFieldValidator = (customValidator, getFieldTitle) => (key, vmSettings, additionalResources) => {
const field = vmSettings[key] || {};
const { validation, ...rest } = customValidator && customValidator(field.value, vmSettings);
const { validation, ...rest } = customValidator && customValidator(field.value, vmSettings, additionalResources);
let val = validation;

// next step is disabled by isValid so empty errors do not need to be shown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ const isTabValidResolver = {
[VM_SETTINGS_TAB_KEY]: isVmSettingsTabValid,
};

export const validateTabData = (tabKey, data, valid) => {
export const validateTabData = (tabKey, data, valid, props) => {
const dataValidator = validateTabResolver[tabKey];
const tabValidator = isTabValidResolver[tabKey];

if (dataValidator) {
data = dataValidator(data);
data = dataValidator(data, props);
}

if (tabValidator) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { get } from 'lodash';

import { isFieldRequired } from '../utils/vmSettingsTabUtils';
import {
getValidationObject,
validateContainer,
validateDNS1123SubdomainValue,
validateURL,
} from '../../../../utils/validations';
import { getValidationObject, validateContainer, validateVmName, validateURL } from '../../../../utils/validations';
import { objectMerge } from '../../../../utils/utils';
import { settingsValue } from '../../../../k8s/selectors';
import { PROVISION_SOURCE_IMPORT, VALIDATION_ERROR_TYPE } from '../../../../constants';
Expand Down Expand Up @@ -41,19 +36,19 @@ const validateProviderDropdown = (key, vmSettings) => {
const asVmSettingsValidator = validator => asGenericFieldValidator(asUpdateValidator(validator), getFieldTitle);

const validateResolver = {
[NAME_KEY]: asVmSettingsValidator(validateDNS1123SubdomainValue),
[NAME_KEY]: asVmSettingsValidator(validateVmName),
[CONTAINER_IMAGE_KEY]: asVmSettingsValidator(validateContainer),
[IMAGE_URL_KEY]: asVmSettingsValidator(validateURL),
[PROVIDER_KEY]: validateProviderDropdown,
};

export const validateVmSettings = vmSettings => {
export const validateVmSettings = (vmSettings, additionalResources) => {
const update = {};

Object.keys(vmSettings).forEach(key => {
const validator = validateResolver[key];
if (validator) {
update[key] = validator(key, vmSettings);
update[key] = validator(key, vmSettings, additionalResources);
}
});

Expand Down
169 changes: 169 additions & 0 deletions src/tests/mocks/vm/vm_validation.mock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
export const vm1 = {
apiVersion: 'kubevirt.io/v1alpha3',
kind: 'VirtualMachine',
metadata: {
clusterName: '',
creationTimestamp: '2018-11-06T14:32:07Z',
generation: 1,
name: 'vm1',
namespace: 'test-namespace',
resourceVersion: '10390764',
selfLink: '/apis/kubevirt.io/v1alpha3/namespaces/default/virtualmachines/vm1',
uid: 'bcc1d0b1-e1d0-11e8-82b4-54ee7586b9c4',
},
status: {
created: true,
ready: true,
},
spec: {
dataVolumeTemplates: [
{
metadata: {
name: 'dv-template',
},
spec: {
source: {
pvc: {
name: 'fooname',
namespace: 'foonamespace',
},
},
pvc: {
accessModes: ['ReadWriteOnce'],
resources: {
requests: {
storage: '1G',
},
},
},
},
},
],
running: true,
template: {
spec: {
domain: {
cpu: { cores: 2 },
devices: {
disks: [
{
disk: {
bus: 'virtio',
},
name: 'rootdisk',
},
],
interfaces: [
{
bridge: {},
name: 'eth0',
},
],
rng: {},
},
resources: {
requests: { memory: '2G' },
},
},
networks: [
{
name: 'eth0',
pod: {},
},
],
terminationGracePeriodSeconds: 0,
volumes: [
{
name: 'rootdisk',
containerDisk: { image: 'kubevirt/cirros-registry-disk-demo' },
},
],
},
},
},
};

export const vm2 = {
apiVersion: 'kubevirt.io/v1alpha3',
kind: 'VirtualMachine',
metadata: {
clusterName: '',
creationTimestamp: '2018-11-06T14:32:07Z',
generation: 1,
name: 'vm2',
namespace: 'test-namespace',
resourceVersion: '10390764',
selfLink: '/apis/kubevirt.io/v1alpha3/namespaces/default/virtualmachines/vm2',
uid: 'bcc1d0b1-e1d0-11e8-82b4-54ee7586b9c5',
},
status: {
created: true,
ready: true,
},
spec: {
dataVolumeTemplates: [
{
metadata: {
name: 'dv-template',
},
spec: {
source: {
pvc: {
name: 'fooname',
namespace: 'foonamespace',
},
},
pvc: {
accessModes: ['ReadWriteOnce'],
resources: {
requests: {
storage: '1G',
},
},
},
},
},
],
running: true,
template: {
spec: {
domain: {
cpu: { cores: 2 },
devices: {
disks: [
{
disk: {
bus: 'virtio',
},
name: 'rootdisk',
},
],
interfaces: [
{
bridge: {},
name: 'eth0',
},
],
rng: {},
},
resources: {
requests: { memory: '2G' },
},
},
networks: [
{
name: 'eth0',
pod: {},
},
],
terminationGracePeriodSeconds: 0,
volumes: [
{
name: 'rootdisk',
containerDisk: { image: 'kubevirt/cirros-registry-disk-demo' },
},
],
},
},
},
};
2 changes: 1 addition & 1 deletion src/utils/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const DNS1123_UPPERCASE_ERROR = 'cannot contain uppercase letters';

export const URL_INVALID_ERROR = 'has to be a valid URL';

export const VIRTUAL_MACHINE_EXISTS = `is already used by another Virtual Machine`;
export const VIRTUAL_MACHINE_EXISTS = `is already used by another virtual machine`;

// export const VMWARE_URL_ERROR = 'vCenter URL is incorrectly formatted. Example: https://host:port/';
export const ERROR = 'Error';
Expand Down
19 changes: 19 additions & 0 deletions src/utils/tests/validation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
validateContainer,
getValidationObject,
validateVmwareURL,
validateVmName,
} from '../validations';
import {
DNS1123_START_ERROR,
Expand All @@ -16,7 +17,11 @@ import {
URL_INVALID_ERROR,
END_WHITESPACE_ERROR,
START_WHITESPACE_ERROR,
VIRTUAL_MACHINE_EXISTS,
} from '../strings';
import { vm1, vm2 } from '../../tests/mocks/vm/vm_validation.mock';
import { validVmSettings } from '../../components/Wizard/CreateVmWizard/fixtures/VmSettingsTab.fixture';
import { NAMESPACE_KEY } from '../../components/Wizard/CreateVmWizard/constants';

const validatesEmpty = validateFunction => {
expect(validateFunction('')).toEqual(getValidationObject(EMPTY_ERROR));
Expand Down Expand Up @@ -125,3 +130,17 @@ describe('validation.js - validateVmwareURL', () => {
expect(validateVmwareURL('http://hello.com ')).toEqual(getValidationObject(END_WHITESPACE_ERROR));
});
});

describe('validation.js - validateVmName', () => {
const props = { virtualMachines: [vm1, vm2] };
const vmSettings = validVmSettings;
vmSettings[NAMESPACE_KEY].value = 'test-namespace';

it('handles unique name', () => {
expect(validateVmName('vm3', vmSettings, props)).toBeNull();
});

it('handles duplicate name', () => {
expect(validateVmName('vm1', vmSettings, props)).toEqual(getValidationObject(VIRTUAL_MACHINE_EXISTS));
});
});
Loading