From 6eb0c447ee8e033b104672809944ec9d05fc18ad Mon Sep 17 00:00:00 2001 From: mathieuRA Date: Mon, 25 Nov 2024 16:46:24 +0100 Subject: [PATCH] changes after first review --- packages/xo-server/src/api/vm.mjs | 11 ++----- packages/xo-server/src/xapi/mixins/vm.mjs | 19 ++++++++---- packages/xo-server/src/xo-mixins/rest-api.mjs | 6 ++-- packages/xo-web/src/xo-app/new-vm/index.js | 30 +++---------------- 4 files changed, 22 insertions(+), 44 deletions(-) diff --git a/packages/xo-server/src/api/vm.mjs b/packages/xo-server/src/api/vm.mjs index e8ca4af549f..9eea57edeb8 100644 --- a/packages/xo-server/src/api/vm.mjs +++ b/packages/xo-server/src/api/vm.mjs @@ -148,16 +148,11 @@ export const create = defer(async function ($defer, params) { params.vifs = vifs && map(vifs, vif => { - if (vif.remove) { - return vif - } - const network = this.getObject(vif.network) objectIds.push(network.id) return { - device: vif.device, mac: vif.mac, network: network._xapiId, ipv4_allowed: vif.allowedIpv4Addresses, @@ -185,7 +180,7 @@ export const create = defer(async function ($defer, params) { params.tags = paramsTags !== undefined ? paramsTags.concat(resourceSetTags) : resourceSetTags } - const xapiVm = await xapi.createVm(template._xapiId, params, checkLimits, user.id) + const xapiVm = await xapi.createVm(template._xapiId, params, checkLimits, user.id, { destroyAllVifs: true }) $defer.onFailure(() => xapi.VM_destroy(xapiVm.$ref, { deleteDisks: true, force: true })) const vm = xapi.xo.addObject(xapiVm) @@ -320,7 +315,7 @@ create.params = { type: 'object', properties: { // UUID of the network to create the interface in. - network: { type: 'string', optional: true }, + network: { type: 'string' }, mac: { optional: true, // Auto-generated per default. @@ -338,8 +333,6 @@ create.params = { type: 'array', items: { type: 'string' }, }, - device: { type: 'string', optional: true }, - remove: { type: 'boolean', optional: true }, }, }, }, diff --git a/packages/xo-server/src/xapi/mixins/vm.mjs b/packages/xo-server/src/xapi/mixins/vm.mjs index 55c4a4e18f3..039a9f13746 100644 --- a/packages/xo-server/src/xapi/mixins/vm.mjs +++ b/packages/xo-server/src/xapi/mixins/vm.mjs @@ -47,7 +47,8 @@ const methods = { ...props } = {}, checkLimits, - creatorId + creatorId, + { destroyAllVifs = false } = {} ) { const installMethod = (() => { if (installRepository == null) { @@ -188,10 +189,16 @@ const methods = { ) } + if (destroyAllVifs) { + // Destroys the VIFs cloned from the template. + await Promise.all(vm.$VIFs.map(vif => this._deleteVif(vif))) + } + + // Creates the VIFs specified by the user. if (vifs) { const devices = await this.call('VM.get_allowed_VIF_devices', vm.$ref) const _vifsToCreate = [] - const _vifsToRemove = [] + const _vifsToDestroy = [] const vmVifByDevice = keyBy(vm.$VIFs, 'device') vifs.forEach(vif => { @@ -202,20 +209,20 @@ const methods = { } const vmVif = vmVifByDevice[vif.device] - if (vif.remove) { + if (vif.destroy) { if (vmVif !== undefined) { - _vifsToRemove.push(vmVif) + _vifsToDestroy.push(vmVif) } return } if (vmVif !== undefined) { - _vifsToRemove.push(vmVif) + _vifsToDestroy.push(vmVif) } _vifsToCreate.push(vif) }) - await Promise.all(_vifsToRemove.map(vif => this._deleteVif(vif))) + await Promise.all(_vifsToDestroy.map(vif => this._deleteVif(vif))) await Promise.all( _vifsToCreate.map(vif => this.VIF_create( diff --git a/packages/xo-server/src/xo-mixins/rest-api.mjs b/packages/xo-server/src/xo-mixins/rest-api.mjs index 4395cc0f07c..9a0b8089116 100644 --- a/packages/xo-server/src/xo-mixins/rest-api.mjs +++ b/packages/xo-server/src/xo-mixins/rest-api.mjs @@ -771,12 +771,12 @@ export default class RestApi { items: { type: 'object', properties: { - remove: { type: 'boolean', optional: true }, - network: { type: 'string', optional: true }, + destroy: { type: 'boolean', optional: true }, device: { type: 'string', optional: true }, - mac: { type: 'string', optional: true }, ipv4_allowed: { type: 'array', items: { type: 'string' }, optional: true }, ipv6_allowed: { type: 'array', items: { type: 'string' }, optional: true }, + mac: { type: 'string', optional: true }, + network: { type: 'string', optional: true }, }, }, }, diff --git a/packages/xo-web/src/xo-app/new-vm/index.js b/packages/xo-web/src/xo-app/new-vm/index.js index b05cfe61964..d7dd108a4a2 100644 --- a/packages/xo-web/src/xo-app/new-vm/index.js +++ b/packages/xo-web/src/xo-app/new-vm/index.js @@ -38,11 +38,9 @@ import { isEmpty, isEqual, join, - keyBy, map, size, slice, - sortBy, sum, sumBy, } from 'lodash' @@ -229,9 +227,6 @@ const isVdiPresent = vdi => !vdi.missing keys => keys ) const getHosts = createGetObjectsOfType('host') - const getTemplateVifs = createGetObjectsOfType('VIF').filter( - createSelector(getTemplate, template => vif => vif.$VM === template?.uuid) - ) return (state, props) => ({ isAdmin: getIsAdmin(state, props), isPoolAdmin: getIsPoolAdmin(state, props), @@ -246,7 +241,6 @@ const isVdiPresent = vdi => !vdi.missing srs: getSrs(state, props), template: getTemplate(state, props, props.pool === undefined), templates: getTemplates(state, props), - templateVifs: getTemplateVifs(state, props), userSshKeys: getUserSshKeys(state, props), hosts: getHosts(state, props), }) @@ -456,8 +450,7 @@ export default class NewVm extends BaseComponent { // Split allowed IPs into IPv4 and IPv6 const { VIFs } = state - const templateVifs = Object.values(this.props.templateVifs) - const _VIFs = map(VIFs, (vif, index) => { + const _VIFs = map(VIFs, vif => { const _vif = { ...vif } if (_vif.mac?.trim() === '') { delete _vif.mac @@ -475,28 +468,12 @@ export default class NewVm extends BaseComponent { _vif.allowedIpv6Addresses.push(ip) } }) - - if (index + 1 <= templateVifs.length) { - _vif.device = String(index) - } return _vif }) const resourceSet = this._getResourceSet() const { template } = this.props - // In case user deletes some VIF created by the template - // We need to mark them with `remove:true` - // so that xo-server deletes them properly - if (_VIFs.length < templateVifs.length) { - const _vifByDevice = keyBy(_VIFs, 'device') - templateVifs.forEach(templateVif => { - if (_vifByDevice[templateVif.device] === undefined) { - _VIFs.push({ remove: true, device: templateVif.device }) - } - }) - } - // Either use `memory` OR `memory*` params let { memory, memoryStaticMax, memoryDynamicMin, memoryDynamicMax } = state if ((memoryStaticMax != null || memoryDynamicMin != null) && memoryDynamicMax == null) { @@ -564,7 +541,7 @@ export default class NewVm extends BaseComponent { const storeState = store.getState() const isInResourceSet = this._getIsInResourceSet() const { state } = this.state - const { pool, templateVifs } = this.props + const { pool } = this.props const resourceSet = this._getResolvedResourceSet() const existingDisks = {} @@ -588,7 +565,8 @@ export default class NewVm extends BaseComponent { const defaultNetworkIds = this._getDefaultNetworkIds(template) forEach( // iterate template VIFs in device order - sortBy(templateVifs, 'device'), + template.VIFs.map(id => getObject(storeState, id, resourceSet)).sort((a, b) => a.device - b.device), + vif => { VIFs.push({ network: pool || isInResourceSet(vif.$network) ? vif.$network : defaultNetworkIds[0],