From 837b06ef2ba13b561a654d2ebd58b62c4dab2ca7 Mon Sep 17 00:00:00 2001 From: Mathieu <70369997+MathieuRA@users.noreply.github.com> Date: Mon, 30 May 2022 11:13:13 +0200 Subject: [PATCH] feat(xo-server/xo-web/pool): avoid RPU/ host reboot, shutdown / host agent reboot during backup (#6232) See zammad#5377 --- @xen-orchestra/backups/Backup.js | 2 +- ...tern.js => extractIdsFromSimplePattern.js} | 0 CHANGELOG.unreleased.md | 1 + packages/xo-server/src/api/_backupGuard.mjs | 31 +++++ packages/xo-server/src/api/host.mjs | 38 +++++- packages/xo-server/src/api/pool.mjs | 18 ++- packages/xo-web/src/common/intl/messages.js | 2 + packages/xo-web/src/common/xo/index.js | 128 +++++++++++++++--- 8 files changed, 193 insertions(+), 27 deletions(-) rename @xen-orchestra/backups/{_extractIdsFromSimplePattern.js => extractIdsFromSimplePattern.js} (100%) create mode 100644 packages/xo-server/src/api/_backupGuard.mjs diff --git a/@xen-orchestra/backups/Backup.js b/@xen-orchestra/backups/Backup.js index 46efbd5b7..2ba45b48b 100644 --- a/@xen-orchestra/backups/Backup.js +++ b/@xen-orchestra/backups/Backup.js @@ -6,7 +6,7 @@ const ignoreErrors = require('promise-toolbox/ignoreErrors') const { compileTemplate } = require('@xen-orchestra/template') const { limitConcurrency } = require('limit-concurrency-decorator') -const { extractIdsFromSimplePattern } = require('./_extractIdsFromSimplePattern.js') +const { extractIdsFromSimplePattern } = require('./extractIdsFromSimplePattern.js') const { PoolMetadataBackup } = require('./_PoolMetadataBackup.js') const { Task } = require('./Task.js') const { VmBackup } = require('./_VmBackup.js') diff --git a/@xen-orchestra/backups/_extractIdsFromSimplePattern.js b/@xen-orchestra/backups/extractIdsFromSimplePattern.js similarity index 100% rename from @xen-orchestra/backups/_extractIdsFromSimplePattern.js rename to @xen-orchestra/backups/extractIdsFromSimplePattern.js diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 99dd318f4..8f33d4480 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -12,6 +12,7 @@ - [Backup] Add setting `backups.metadata.defaultSettings.unconditionalSnapshot` in `xo-server`'s configuration file to force a snapshot even when not required by the backup, this is useful to avoid locking the VM halted during the backup (PR [#6221](https://github.com/vatesfr/xen-orchestra/pull/6221)) - [XO Web] Add ability to configure a default filter for Storage [#6236](https://github.com/vatesfr/xen-orchestra/issues/6236) (PR [#6237](https://github.com/vatesfr/xen-orchestra/pull/6237)) - [Backup] VMs with USB Pass-through devices are now supported! The advanced _Offline Snapshot Mode_ setting must be enabled. For Full Backup or Disaster Recovery jobs, Rolling Snapshot needs to be anabled as well. (PR [#6239](https://github.com/vatesfr/xen-orchestra/pull/6239)) +- [RPU/Host] If some backup jobs are running on the pool, ask for confirmation before starting an RPU, shutdown/rebooting a host or restarting a host's toolstack (PR [6232](https://github.com/vatesfr/xen-orchestra/pull/6232)) ### Bug fixes diff --git a/packages/xo-server/src/api/_backupGuard.mjs b/packages/xo-server/src/api/_backupGuard.mjs new file mode 100644 index 000000000..724db629e --- /dev/null +++ b/packages/xo-server/src/api/_backupGuard.mjs @@ -0,0 +1,31 @@ +import { createPredicate } from 'value-matcher' +import { extractIdsFromSimplePattern } from '@xen-orchestra/backups/extractIdsFromSimplePattern.js' +import { forbiddenOperation } from 'xo-common/api-errors.js' + +export default async function backupGuard(poolId) { + const jobs = await this.getAllJobs('backup') + const guard = id => { + if (this.getObject(id).$poolId === poolId) { + throw forbiddenOperation('Backup is running', `A backup is running on the pool: ${poolId}`) + } + } + + jobs.forEach(({ runId, vms }) => { + // If runId is undefined, the job is not currently running. + if (runId !== undefined) { + if (vms.id !== undefined) { + extractIdsFromSimplePattern(vms).forEach(guard) + } else { + // smartmode + // For the smartmode we take a simplified approach : + // if the smartmode is explicitly 'resident' or 'not resident' on pools : we check if it concern this pool + // if not, the job may concern this pool and we show the warning without looking through all the impacted VM + + const isPoolSafe = vms.$pool === undefined ? false : !createPredicate(vms.$pool)(poolId) + if (!isPoolSafe) { + throw forbiddenOperation('May have running backup', `A backup may run on the pool: ${poolId}`) + } + } + } + }) +} diff --git a/packages/xo-server/src/api/host.mjs b/packages/xo-server/src/api/host.mjs index 164dffa6e..fac8b562e 100644 --- a/packages/xo-server/src/api/host.mjs +++ b/packages/xo-server/src/api/host.mjs @@ -1,6 +1,11 @@ +import { createLogger } from '@xen-orchestra/log' import assert from 'assert' import { format } from 'json-rpc-peer' +import backupGuard from './_backupGuard.mjs' + +const log = createLogger('xo:api:host') + // =================================================================== export function setMaintenanceMode({ host, maintenance }) { @@ -113,13 +118,22 @@ set.resolve = { // FIXME: set force to false per default when correctly implemented in // UI. -export function restart({ host, force = true }) { +export async function restart({ bypassBackupCheck = false, host, force = true }) { + if (bypassBackupCheck) { + log.warn('host.restart with argument "bypassBackupCheck" set to true', { hostId: host.id }) + } else { + await backupGuard.call(this, host.$poolId) + } return this.getXapi(host).rebootHost(host._xapiId, force) } restart.description = 'restart the host' restart.params = { + bypassBackupCheck: { + type: 'boolean', + optional: true, + }, id: { type: 'string' }, force: { type: 'boolean', @@ -133,13 +147,22 @@ restart.resolve = { // ------------------------------------------------------------------- -export function restartAgent({ host }) { +export async function restartAgent({ bypassBackupCheck = false, host }) { + if (bypassBackupCheck) { + log.warn('host.restartAgent with argument "bypassBackupCheck" set to true', { hostId: host.id }) + } else { + await backupGuard.call(this, host.$poolId) + } return this.getXapiObject(host).$restartAgent() } restartAgent.description = 'restart the Xen agent on the host' restartAgent.params = { + bypassBackupCheck: { + type: 'boolean', + optional: true, + }, id: { type: 'string' }, } @@ -183,13 +206,22 @@ start.resolve = { // ------------------------------------------------------------------- -export function stop({ host, bypassEvacuate }) { +export async function stop({ bypassBackupCheck = false, host, bypassEvacuate }) { + if (bypassBackupCheck) { + log.warn('host.stop with argument "bypassBackupCheck" set to true', { hostId: host.id }) + } else { + await backupGuard.call(this, host.$poolId) + } return this.getXapi(host).shutdownHost(host._xapiId, { bypassEvacuate }) } stop.description = 'stop the host' stop.params = { + bypassBackupCheck: { + type: 'boolean', + optional: true, + }, id: { type: 'string' }, bypassEvacuate: { type: 'boolean', optional: true }, } diff --git a/packages/xo-server/src/api/pool.mjs b/packages/xo-server/src/api/pool.mjs index 6017f9044..d5408b149 100644 --- a/packages/xo-server/src/api/pool.mjs +++ b/packages/xo-server/src/api/pool.mjs @@ -1,11 +1,16 @@ import { asyncMap } from '@xen-orchestra/async-map' +import { createLogger } from '@xen-orchestra/log' import { defer as deferrable } from 'golike-defer' import { format } from 'json-rpc-peer' import { Ref } from 'xen-api' import { incorrectState } from 'xo-common/api-errors.js' +import backupGuard from './_backupGuard.mjs' + import { moveFirst } from '../_moveFirst.mjs' +const log = createLogger('xo:api:pool') + // =================================================================== export async function set({ @@ -162,7 +167,14 @@ installPatches.description = 'Install patches on hosts' // ------------------------------------------------------------------- -export const rollingUpdate = deferrable(async function ($defer, { pool }) { +export const rollingUpdate = deferrable(async function ($defer, { bypassBackupCheck = false, pool }) { + const poolId = pool.id + if (bypassBackupCheck) { + log.warn('pool.rollingUpdate update with argument "bypassBackupCheck" set to true', { poolId }) + } else { + await backupGuard.call(this, poolId) + } + if ((await this.getOptionalPlugin('load-balancer'))?.loaded) { await this.unloadPlugin('load-balancer') $defer(() => this.loadPlugin('load-balancer')) @@ -172,6 +184,10 @@ export const rollingUpdate = deferrable(async function ($defer, { pool }) { }) rollingUpdate.params = { + bypassBackupCheck: { + optional: true, + type: 'boolean', + }, pool: { type: 'string' }, } diff --git a/packages/xo-web/src/common/intl/messages.js b/packages/xo-web/src/common/intl/messages.js index ebfa5c1a1..89c66f433 100644 --- a/packages/xo-web/src/common/intl/messages.js +++ b/packages/xo-web/src/common/intl/messages.js @@ -1688,6 +1688,8 @@ const messages = { restoreFilesUnselectAll: 'Unselect all files', // ----- Modals ----- + bypassBackupHostModalMessage: 'There may be ongoing backups on the host. Are you sure you want to continue?', + bypassBackupPoolModalMessage: 'There may be ongoing backups on the pool. Are you sure you want to continue?', emergencyShutdownHostModalTitle: 'Emergency shutdown Host', emergencyShutdownHostModalMessage: 'Are you sure you want to shutdown {host}?', emergencyShutdownHostsModalTitle: 'Emergency shutdown Host{nHosts, plural, one {} other {s}}', diff --git a/packages/xo-web/src/common/xo/index.js b/packages/xo-web/src/common/xo/index.js index ad419c700..6a55de881 100644 --- a/packages/xo-web/src/common/xo/index.js +++ b/packages/xo-web/src/common/xo/index.js @@ -766,11 +766,34 @@ export const restartHost = (host, force = false) => body: _('restartHostModalMessage'), }).then( () => - _call('host.restart', { id: resolveId(host), force }).catch(error => { - if (noHostsAvailable.is(error)) { - alert(_('noHostsAvailableErrorTitle'), _('noHostsAvailableErrorMessage')) - } - }), + _call('host.restart', { id: resolveId(host), force }) + .catch(async error => { + if ( + forbiddenOperation.is(error, { + reason: `A backup may run on the pool: ${host.$poolId}`, + }) || + forbiddenOperation.is(error, { + reason: `A backup is running on the pool: ${host.$poolId}`, + }) + ) { + await confirm({ + body: ( +

+ {_('bypassBackupHostModalMessage')} +

+ ), + title: _('restartHostModalTitle'), + }) + return _call('host.restart', { id: resolveId(host), force, ignoreBackup: true }) + } + throw error + }) + .catch(error => { + if (noHostsAvailable.is(error)) { + alert(_('noHostsAvailableErrorTitle'), _('noHostsAvailableErrorMessage')) + } + throw error + }), noop ) @@ -799,7 +822,24 @@ export const restartHosts = (hosts, force = false) => { ) } -export const restartHostAgent = host => _call('host.restart_agent', { id: resolveId(host) }) +export const restartHostAgent = async host => { + try { + await _call('host.restart_agent', { id: resolveId(host) }) + } catch (error) { + if (forbiddenOperation.is(error)) { + await confirm({ + body: ( +

+ {_('bypassBackupHostModalMessage')} +

+ ), + title: _('restartHostAgent'), + }) + return _call('host.restart_agent', { id: resolveId(host), ignoreBackup: true }) + } + throw error + } +} export const restartHostsAgents = hosts => { const nHosts = size(hosts) @@ -817,19 +857,41 @@ export const stopHost = async host => { title: _('stopHostModalTitle'), }) - try { - await _call('host.stop', { id: resolveId(host) }) - } catch (err) { - if (err.message === 'no hosts available') { - // Retry with bypassEvacuate. - await confirm({ - body: _('forceStopHostMessage'), - title: _('forceStopHost'), - }) - return _call('host.stop', { id: resolveId(host), bypassEvacuate: true }) - } - throw error - } + let ignoreBackup = false + return _call('host.stop', { id: resolveId(host) }) + .catch(async err => { + if ( + forbiddenOperation.is(err, { + reason: `A backup may run on the pool: ${host.$poolId}`, + }) || + forbiddenOperation.is(error, { + reason: `A backup is running on the pool: ${host.$poolId}`, + }) + ) { + ignoreBackup = true + await confirm({ + body: ( +

+ {_('bypassBackupHostModalMessage')} +

+ ), + title: _('stopHostModalTitle'), + }) + return _call('host.stop', { id: resolveId(host), ignoreBackup }) + } + throw err + }) + .catch(async err => { + if (noHostsAvailable.is(err)) { + await confirm({ + body: _('forceStopHostMessage'), + title: _('forceStopHost'), + }) + // Retry with bypassEvacuate. + return _call('host.stop', { id: resolveId(host), bypassEvacuate: true, ignoreBackup }) + } + throw err + }) } export const stopHosts = hosts => { @@ -946,9 +1008,31 @@ export const rollingPoolUpdate = poolId => body: , title: _('rollingPoolUpdate'), icon: 'pool-rolling-update', - }).then( - () => _call('pool.rollingUpdate', { pool: poolId })::tap(() => subscribeHostMissingPatches.forceRefresh()), - noop + }).then(() => + _call('pool.rollingUpdate', { pool: poolId })::tap( + () => subscribeHostMissingPatches.forceRefresh(), + err => { + if (!forbiddenOperation.is(err)) { + throw err + } + confirm({ + body: ( +

+ {_('bypassBackupPoolModalMessage')} +

+ ), + title: _('rollingPoolUpdate'), + icon: 'pool-rolling-update', + }).then( + () => + _call('pool.rollingUpdate', { ignoreBackup: true, pool: poolId })::tap(() => + subscribeHostMissingPatches.forceRefresh() + ), + noop + ) + }, + noop + ) ) export const installSupplementalPack = (host, file) => {