fix(xo-server): automatically reconnect to XenServer after connection (#10086)

A server left enabled but without a Xapi instance was never retried by anything in
xo-server. Three paths lead to that state:

    a failed explicit connect: xo-server boots while a pool is down, a user clicks
    "connect" during an outage, or the host-eject path exhausts its 5 attempts;
    a SESSION_INVALID teardown after a pool master restart (_sessionCall calls
    disconnect(), and nothing listens to server:disconnected);
    fixing a server's credentials/address while it is disconnected (server.set never
    triggers a connection).

Once there, the server stays disconnected forever, showing an error that can be
minutes or hours old. Support ends up chasing false "pool is dead" reports, typically
right after a rolling pool update, which is exactly when hosts reboot by design.
This commit is contained in:
Mathieu
2026-07-20 09:42:12 +02:00
committed by GitHub
parent 926eb348d3
commit 5e6c4ad96d
3 changed files with 284 additions and 5 deletions

View File

@@ -0,0 +1,61 @@
import { fibonacci } from 'iterable-backoff'
// hard cap between two attempts: quick first retries, then steady 1/min
export const MAX_DELAY = 60e3
// Reconnection loop for a server in the `enabled but disconnected` state,
// which nothing else in xo-server ever retries: a failed explicit connect
// (boot, user action, host eject) or an unexpected XAPI disconnection tears
// the Xapi instance down and, without this loop, the server would stay
// disconnected until a manual reconnection.
//
// Deps are injected to keep this testable without a full app:
// - connect(id): attempt the connection, throws on failure
// - delay(ms): wait before the next attempt
// - getServer(id): server record ({ enabled }), throws if the server was deleted
// - getStatus(id): 'disconnected' | 'connecting' | 'connected'
// - isFatal(error): optional, true for permanent errors not worth retrying
// - isGone(error): optional, true when a getServer error means the server was deleted
//
// Resolves with the reason why the loop stopped, never rejects.
export async function autoReconnect(
id,
{ connect, delay, getServer, getStatus, isFatal = () => false, isGone = () => true, log }
) {
for (const ms of fibonacci()
.toMs()
.map(ms => Math.min(ms, MAX_DELAY))) {
await delay(ms)
let server
try {
server = await getServer(id)
} catch (error) {
if (isGone(error)) {
return 'server deleted'
}
// transient failure (e.g. database hiccup): keep the loop alive
log.debug('auto-reconnect could not read the server', { serverId: id, error })
continue
}
if (!server.enabled) {
return 'server disabled'
}
if (getStatus(id) !== 'disconnected') {
return 'already connected'
}
try {
await connect(id)
return 'connected'
} catch (error) {
// retrying would hammer the host with invalid credentials, or never
// succeed for permanent errors (e.g. pool already connected)
if (error?.code === 'SESSION_AUTHENTICATION_FAILED' || isFatal(error)) {
log.warn('auto-reconnect aborted: permanent error', { serverId: id, error })
return 'permanent error'
}
log.debug('auto-reconnect attempt failed', { serverId: id, error })
}
}
}

View File

@@ -0,0 +1,149 @@
import assert from 'node:assert/strict'
import { describe, it } from 'node:test'
import { autoReconnect, MAX_DELAY } from './_xenServerAutoReconnect.mjs'
const noopLog = { debug: () => {}, warn: () => {} }
function makeDeps(overrides) {
return {
connect: async () => {},
delay: () => Promise.resolve(),
getServer: async () => ({ enabled: true }),
getStatus: () => 'disconnected',
log: noopLog,
...overrides,
}
}
describe('autoReconnect', () => {
it('reconnects after transient failures', async () => {
let attempts = 0
const outcome = await autoReconnect(
's1',
makeDeps({
connect: async () => {
if (++attempts < 3) {
throw new Error('EHOSTUNREACH')
}
},
})
)
assert.equal(outcome, 'connected')
assert.equal(attempts, 3)
})
it('stops when the server is disabled', async () => {
const outcome = await autoReconnect('s1', makeDeps({ getServer: async () => ({ enabled: false }) }))
assert.equal(outcome, 'server disabled')
})
it('stops when the server was deleted', async () => {
const outcome = await autoReconnect(
's1',
makeDeps({
getServer: async () => {
throw new Error('no such object')
},
isGone: error => error.message === 'no such object',
})
)
assert.equal(outcome, 'server deleted')
})
it('survives transient getServer failures when isGone says the server still exists', async () => {
let reads = 0
let attempts = 0
const outcome = await autoReconnect(
's1',
makeDeps({
getServer: async () => {
if (++reads === 1) {
throw new Error('database hiccup')
}
return { enabled: true }
},
isGone: () => false,
connect: async () => {
++attempts
},
})
)
assert.equal(outcome, 'connected')
assert.equal(reads, 2)
assert.equal(attempts, 1)
})
it('stops when the server was reconnected by other means', async () => {
let connectCalled = false
const outcome = await autoReconnect(
's1',
makeDeps({
getStatus: () => 'connected',
connect: async () => {
connectCalled = true
},
})
)
assert.equal(outcome, 'already connected')
assert.equal(connectCalled, false)
})
it('gives up immediately on authentication failure', async () => {
let attempts = 0
const outcome = await autoReconnect(
's1',
makeDeps({
connect: async () => {
++attempts
const error = new Error('authentication failed')
error.code = 'SESSION_AUTHENTICATION_FAILED'
throw error
},
})
)
assert.equal(outcome, 'permanent error')
assert.equal(attempts, 1)
})
it('gives up immediately on errors reported fatal by isFatal', async () => {
let attempts = 0
const outcome = await autoReconnect(
's1',
makeDeps({
connect: async () => {
++attempts
throw new Error('pool already connected')
},
isFatal: error => error.message === 'pool already connected',
})
)
assert.equal(outcome, 'permanent error')
assert.equal(attempts, 1)
})
it('starts fast and caps delays at MAX_DELAY', async () => {
const delays = []
let attempts = 0
const outcome = await autoReconnect(
's1',
makeDeps({
delay: async ms => {
delays.push(ms)
},
connect: async () => {
if (++attempts < 15) {
throw new Error('still down')
}
},
})
)
assert.equal(outcome, 'connected')
assert.ok(delays[0] <= 2e3, `first delay should be short, got ${delays[0]}`)
assert.ok(
delays.every(ms => ms <= MAX_DELAY),
'delays must be capped'
)
assert.equal(Math.max(...delays), MAX_DELAY)
})
})

View File

@@ -21,6 +21,7 @@ import { acquireRpuGuard } from '../_rpuGuard.mjs'
import { getRpuTracesConfig, openRpuTrace } from '../_rpuObservability.mjs'
import xapiObjectToXo from '../xapi-object-to-xo.mjs'
import XapiStats from '../xapi-stats.mjs'
import { autoReconnect } from '../_xenServerAutoReconnect.mjs'
import { camelToSnakeCase, forEach, isEmpty, popProperty } from '../utils.mjs'
import { Servers } from '../models/server.mjs'
@@ -49,6 +50,8 @@ const log = createLogger('xo:xo-mixins:xen-servers')
export default class XenServers {
constructor(app, { safeMode }) {
this._objectConflicts = { __proto__: null } // TODO: clean when a server is disconnected.
this._connectingXenServers = new Set()
this._reconnectingXenServers = new Set()
this._serverIdsByPool = { __proto__: null }
this._stats = new XapiStats()
this._xapis = { __proto__: null }
@@ -223,6 +226,12 @@ export default class XenServers {
if (hasChanged) {
await this._servers.update(server)
}
// an enabled server must not stay disconnected without retries, e.g. after
// the user fixed its credentials or address
if (server.enabled && !this._connectingXenServers.has(id) && this._getXenServerStatus(id) === 'disconnected') {
this._autoReconnectXenServer(id)
}
}
async getXenServerWithCredentials(id) {
@@ -358,18 +367,63 @@ export default class XenServers {
})
}
async connectXenServer(id) {
// starts a reconnection loop for a server left `enabled` but without a
// Xapi instance, a state nothing else retries (see _xenServerAutoReconnect.mjs)
_autoReconnectXenServer(id) {
const reconnecting = this._reconnectingXenServers
if (reconnecting.has(id)) {
return
}
reconnecting.add(id)
autoReconnect(id, {
// `enable: false`: the loop must never overwrite a concurrent disable
connect: id => this.connectXenServer(id, { enable: false }),
delay: pDelay,
getServer: id => this.getXenServer(id),
getStatus: id => this._getXenServerStatus(id),
isFatal: error => error instanceof PoolAlreadyConnected,
isGone: error => noSuchObject.is(error),
log,
})
.then(outcome => {
log.info('auto-reconnect stopped', { serverId: id, outcome })
})
.catch(error => {
log.error('auto-reconnect crashed', { serverId: id, error })
})
.finally(() => {
reconnecting.delete(id)
})
}
async connectXenServer(id, { enable = true } = {}) {
const server = await this.getXenServerWithCredentials(id)
const serverStatus = this._getXenServerStatus(id)
if (serverStatus !== 'disconnected') {
// `_connectingXenServers` also guards against a concurrent connection
// attempt for the same server, which would overwrite `_xapis[id]` and leak
// a live connection
const connecting = this._connectingXenServers.has(id)
if (serverStatus !== 'disconnected' || connecting) {
throw incorrectState({
actual: serverStatus,
actual: connecting ? 'connecting' : serverStatus,
expected: 'disconnected',
object: server.id,
property: 'status',
})
}
this._connectingXenServers.add(id)
try {
await this._connectXenServer(id, server, { enable })
} finally {
this._connectingXenServers.delete(id)
}
}
async _connectXenServer(id, server, { enable }) {
if (enable) {
await this.updateXenServer(id, { enabled: true })
}
const { config } = this._app
@@ -572,12 +626,27 @@ export default class XenServers {
delete this._xapis[server.id]
delete this._serverIdsByPool[poolId]
this._app.emit('server:disconnected', { server, xapi })
// deliberate disconnections set `enabled` to false beforehand, in
// which case the loop stops on its own
this._autoReconnectXenServer(server.id)
})
this._app.emit('server:connected', { server, xapi })
} catch (error) {
delete this._xapis[server.id]
xapi.disconnect()::ignoreErrors()
// avoid a database write per auto-reconnect attempt when the error did not change
const previousError = server.error
if (previousError?.code !== error?.code || previousError?.message !== error?.message) {
this.updateXenServer(id, { error })::ignoreErrors()
}
// permanent errors: retrying is pointless, do not start the loop
if (!(error instanceof PoolAlreadyConnected) && error?.code !== 'SESSION_AUTHENTICATION_FAILED') {
this._autoReconnectXenServer(id)
}
throw error
}
}