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

Commit

Permalink
Display error on duplicate VM name in wizard (#449)
Browse files Browse the repository at this point in the history
  • Loading branch information
@suomiy @rawagner
suomiy authored and rawagner committed May 15, 2019
1 parent d78e9ff commit 8a19ee6
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 20 deletions.
12 changes: 8 additions & 4 deletions src/components/Wizard/CreateVmWizard/CreateVmWizard.js
View file
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
View file
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';
4 changes: 4 additions & 0 deletions src/components/Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture.js
View file
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: [],
},
},
];
3 changes: 2 additions & 1 deletion src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js
View file
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
View file
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
4 changes: 2 additions & 2 deletions src/components/Wizard/CreateVmWizard/validations/validators.js
View file
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
13 changes: 4 additions & 9 deletions src/components/Wizard/CreateVmWizard/validations/vmSettingsTabValidation.js
View file
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
View file
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
View file
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
View file
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

0 comments on commit 8a19ee6

Please sign in to comment.