Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(xo-server): ability to manage VIFs using REST API when creating a VM #8137

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Ignore leading and trailing spaces when editing VM/Pools/Hosts/SRs names and descriptions (PR [#8115](https://github.com/vatesfr/xen-orchestra/pull/8115))
- [VM/Advanced] in Nested virtualization section, add warning tooltip and link to documentation (PR [#8107](https://github.com/vatesfr/xen-orchestra/pull/8107))
- [REST/VM] When creating a VM, the template's VIFs are created. It is also possible to create more VIFs or delete/update template's VIFs (PR [#8137](https://github.com/vatesfr/xen-orchestra/pull/8137))

### Bug fixes

Expand Down Expand Up @@ -40,6 +41,7 @@
- @xen-orchestra/web patch
- @xen-orchestra/web-core minor
- @xen-orchestra/xapi patch
- xo-server minor
- xo-web minor

<!--packages-end-->
9 changes: 8 additions & 1 deletion packages/xo-server/src/api/vm.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,16 @@ 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,
Expand Down Expand Up @@ -315,7 +320,7 @@ create.params = {
type: 'object',
properties: {
// UUID of the network to create the interface in.
network: { type: 'string' },
network: { type: 'string', optional: true },

mac: {
optional: true, // Auto-generated per default.
Expand All @@ -333,6 +338,8 @@ create.params = {
type: 'array',
items: { type: 'string' },
},
device: { type: 'string', optional: true },
remove: { type: 'boolean', optional: true },
},
},
},
Expand Down
35 changes: 29 additions & 6 deletions packages/xo-server/src/xapi/mixins/vm.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import find from 'lodash/find.js'
import gte from 'lodash/gte.js'
import includes from 'lodash/includes.js'
import isEmpty from 'lodash/isEmpty.js'
import keyBy from 'lodash/keyBy.js'
import lte from 'lodash/lte.js'
import mapToArray from 'lodash/map.js'
import mapValues from 'lodash/mapValues.js'
Expand Down Expand Up @@ -187,19 +188,41 @@ const methods = {
)
}

// 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 vmVifByDevice = keyBy(vm.$VIFs, 'device')

vifs.forEach(vif => {
if (vif.device === undefined) {
vif.device = devices.shift()
_vifsToCreate.push(vif)
return
}

const vmVif = vmVifByDevice[vif.device]
if (vif.remove) {
if (vmVif !== undefined) {
_vifsToRemove.push(vmVif)
}
return
}

if (vmVif !== undefined) {
_vifsToRemove.push(vmVif)
}
_vifsToCreate.push(vif)
})

await Promise.all(_vifsToRemove.map(vif => this._deleteVif(vif)))
await Promise.all(
mapToArray(vifs, (vif, index) =>
_vifsToCreate.map(vif =>
this.VIF_create(
{
ipv4_allowed: vif.ipv4_allowed,
ipv6_allowed: vif.ipv6_allowed,
device: devices[index],
device: vif.device,
locking_mode: isEmpty(vif.ipv4_allowed) && isEmpty(vif.ipv6_allowed) ? 'network_default' : 'locked',
MTU: vif.mtu,
network: this.getObject(vif.network).$ref,
Expand Down
15 changes: 15 additions & 0 deletions packages/xo-server/src/xo-mixins/rest-api.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,21 @@ export default class RestApi {
name_label: { type: 'string' },
network_config: { type: 'string', optional: true },
template: { type: 'string' },
vifs: {
default: [],
type: 'array',
items: {
type: 'object',
properties: {
remove: { type: 'boolean', optional: true },
network: { type: 'string', 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 },
},
},
},
}
),
emergency_shutdown: async ({ xapiObject }) => {
Expand Down
30 changes: 26 additions & 4 deletions packages/xo-web/src/xo-app/new-vm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ import {
isEmpty,
isEqual,
join,
keyBy,
map,
size,
slice,
sortBy,
sum,
sumBy,
} from 'lodash'
Expand Down Expand Up @@ -227,6 +229,9 @@ 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),
Expand All @@ -241,6 +246,7 @@ 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),
})
Expand Down Expand Up @@ -450,7 +456,8 @@ export default class NewVm extends BaseComponent {

// Split allowed IPs into IPv4 and IPv6
const { VIFs } = state
const _VIFs = map(VIFs, vif => {
const templateVifs = Object.values(this.props.templateVifs)
const _VIFs = map(VIFs, (vif, index) => {
const _vif = { ...vif }
if (_vif.mac?.trim() === '') {
delete _vif.mac
Expand All @@ -468,12 +475,28 @@ export default class NewVm extends BaseComponent {
_vif.allowedIpv6Addresses.push(ip)
}
})

if (index + 1 <= templateVifs.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more simple like this?

Suggested change
if (index + 1 <= templateVifs.length) {
if (index < 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) {
Expand Down Expand Up @@ -541,7 +564,7 @@ export default class NewVm extends BaseComponent {
const storeState = store.getState()
const isInResourceSet = this._getIsInResourceSet()
const { state } = this.state
const { pool } = this.props
const { pool, templateVifs } = this.props
const resourceSet = this._getResolvedResourceSet()

const existingDisks = {}
Expand All @@ -565,8 +588,7 @@ export default class NewVm extends BaseComponent {
const defaultNetworkIds = this._getDefaultNetworkIds(template)
forEach(
// iterate template VIFs in device order
template.VIFs.map(id => getObject(storeState, id, resourceSet)).sort((a, b) => a.device - b.device),

sortBy(templateVifs, 'device'),
vif => {
VIFs.push({
network: pool || isInResourceSet(vif.$network) ? vif.$network : defaultNetworkIds[0],
Expand Down
Loading