fix(xo-server-sdn-controller): clean up partial state on network registration failure

When privateNetwork.addNetwork() fails during xapi connection, the network is 
already inserted in privateNetwork.networks and a center may have been set 
without being recorded in _starCenters, leaving a broken PrivateNetwork 
registered.

After the registration loop, remove the network that failed to register, drop 
the orphaned center, and delete the PrivateNetwork once it has no network left, 
like _handleDisconnectedXapi already does.

Also defer pushing to noVniNetworks until the network is registered, so a failed 
network gets no VNI and does not trigger _electNewCenter outside of any 
try/catch.

packages/xo-server-sdn-controller/src/index.js

@@ -515,6 +515,7 @@ class SDNController extends EventEmitter {
       // Add already existing private networks
       const networks = Object.values(xapi.objects.indexes.type.network ?? {})
       const noVniNetworks = []
+      const registeredNetworks = new Set()
       await Promise.all(
         networks.map(async network => {
           // 2019-09-03
@@ -557,18 +558,23 @@ class SDNController extends EventEmitter {
 
           try {
             const vni = otherConfig['xo:sdn-controller:vni']
-            if (vni === undefined) {
-              noVniNetworks.push(network)
-            } else {
+            if (vni !== undefined) {
               this._prevVni = Math.max(this._prevVni, +vni)
             }
 
             await privateNetwork.addNetwork(network)
 
             this._networks.set(network.$id, network.$ref)
+            registeredNetworks.add(network.$ref)
             if (privateNetwork.center !== undefined) {
               this._starCenters.set(privateNetwork.center.$id, privateNetwork.center.$ref)
             }
+
+            // Only networks successfully registered get a VNI: a failed
+            // addNetwork must not reach the re-election loop below
+            if (vni === undefined) {
+              noVniNetworks.push(network)
+            }
           } catch (error) {
             log.warn('Error registering network for SDN controller', {
               network: network.name_label,
@@ -578,6 +584,30 @@ class SDNController extends EventEmitter {
         })
       )
 
+      // Undo the partial state left by failed addNetwork calls: the network
+      // has already been inserted in privateNetwork.networks, and a center
+      // may have been set without being recorded in this._starCenters
+      forOwn(this.privateNetworks, (privateNetwork, uuid) => {
+        const network = privateNetwork.networks[xapi.pool.uuid]
+        if (network === undefined || registeredNetworks.has(network.$ref)) {
+          return
+        }
+
+        delete privateNetwork.networks[xapi.pool.uuid]
+        if (
+          privateNetwork.center !== undefined &&
+          privateNetwork.networks[privateNetwork.center.$pool.uuid] === undefined
+        ) {
+          // The election was never finished: drop the orphaned center. No
+          // this._starCenters cleanup is needed as it was only set on success
+          delete privateNetwork.center
+        }
+
+        if (Object.keys(privateNetwork.networks).length === 0) {
+          delete this.privateNetworks[uuid]
+        }
+      })
+
       // Add VNI to other config of networks without VNI
       //
       // 2019-08-22

packages/xo-server-sdn-controller/src/index.test.js

@@ -233,4 +233,68 @@ describe('SDN controller reliability', () => {
       await controller._pifUpdated(pif)
     })
   })
+
+  it('cleans up partial state when addNetwork fails', async () => {
+    const controller = createFakeController()
+
+    const networkUuid = 'network-uuid-1'
+    const poolUuid = 'pool-uuid-1'
+    const networkId = 'network-1'
+    const xapi = createFakeXapi()
+
+    const createHost = hostId => ({
+      $ref: `host-ref-${hostId}`,
+      $id: hostId,
+      uuid: hostId,
+      name_label: hostId,
+      enabled: true,
+      $metrics: { live: true },
+      $pool: { uuid: poolUuid, $ref: `pool-ref-${poolUuid}`, $id: `${poolUuid}-id` },
+      // host-1 has no PIF on the private network: addHost fails on it,
+      // while host-2 is eligible and gets elected center
+      $PIFs: hostId === 'host-2' ? [{ network: `network-ref-${networkId}`, currently_attached: true }] : [],
+    })
+    const host1 = createHost('host-1')
+    const host2 = createHost('host-2')
+
+    const testNetwork = {
+      $ref: `network-ref-${networkId}`,
+      $id: networkId,
+      name_label: 'test-network',
+      other_config: {
+        'xo:sdn-controller:private-network-uuid': networkUuid,
+        'xo:sdn-controller:vni': '100',
+      },
+      $PIFs: [{ $tunnel_access_PIF_of: [{ $transport_PIF: { ip_configuration_mode: 'static' } }] }],
+      $pool: { uuid: poolUuid, $ref: `pool-ref-${poolUuid}`, $id: `${poolUuid}-id`, $xapi: xapi },
+      $xapi: xapi,
+    }
+
+    xapi.objects.indexes = {
+      type: {
+        SDN_controller: [{ protocol: 'pssl', address: '', port: 0 }],
+        network: [testNetwork],
+        host: [host1, host2],
+      },
+    }
+    xapi.barrier = async ref => (ref === testNetwork.$ref ? testNetwork : undefined)
+    xapi.getRecord = async () => testNetwork
+
+    // OVSDB clients that reset fine; the addNetwork failure comes from the
+    // missing PIF of host-1, leaving the elected center (host-2) set without
+    // being registered in _starCenters
+    const ovsdbClient = { resetForNetwork: async () => undefined }
+    controller.ovsdbClients = { [host1.$ref]: ovsdbClient, [host2.$ref]: ovsdbClient }
+
+    controller._getOrCreateOvsdbClient = mock.fn(() => undefined)
+    controller._getOrCreateOfChannel = mock.fn(() => undefined)
+
+    await assert.doesNotReject(async () => {
+      await controller._handleConnectedXapi(xapi)
+    })
+
+    assert.equal(controller.privateNetworks[networkUuid], undefined)
+    assert.equal(controller._networks.size, 0)
+    assert.equal(controller._starCenters.size, 0)
+  })
 })