From b67231c56bb1c19d8b3f35bddd6a3b3fb151c6b2 Mon Sep 17 00:00:00 2001
From: badrAZ <azizbibadr@gmail.com>
Date: Thu, 18 Oct 2018 10:23:56 +0200
Subject: [PATCH] feat(xo-server/backup-ng): report missing VMs (#3522)

Fixes #3434
---
 CHANGELOG.md                                  |  1 +
 packages/xo-common/src/api-errors.js          |  2 +-
 .../src/xo-mixins/backups-ng-logs.js          |  3 +-
 .../src/xo-mixins/backups-ng/index.js         | 84 +++++++++++++++----
 4 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 005e9338a..fd1762819 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - [Settings/acls] Add bulk deletion [#3179](https://github.com/vatesfr/xen-orchestra/issues/3179) (PR [#3536](https://github.com/vatesfr/xen-orchestra/pull/3536))
 - [Home] Improve search usage: raw numbers also match in names [#2906](https://github.com/vatesfr/xen-orchestra/issues/2906) (PR [#3552](https://github.com/vatesfr/xen-orchestra/pull/3552))
 - [Backup NG] Timeout of a job is now in hours [#3550](https://github.com/vatesfr/xen-orchestra/issues/3550) (PR [#3553](https://github.com/vatesfr/xen-orchestra/pull/3553))
+- [Backup NG] Explicit error if a VM is missing [#3434](https://github.com/vatesfr/xen-orchestra/issues/3434) (PR [#3522](https://github.com/vatesfr/xen-orchestra/pull/3522))
 
 ### Bug fixes
 
diff --git a/packages/xo-common/src/api-errors.js b/packages/xo-common/src/api-errors.js
index 10dec8e2e..8766f292e 100644
--- a/packages/xo-common/src/api-errors.js
+++ b/packages/xo-common/src/api-errors.js
@@ -34,7 +34,7 @@ export const notImplemented = create(0, () => ({
 
 export const noSuchObject = create(1, (id, type) => ({
   data: { id, type },
-  message: 'no such object',
+  message: `no such ${type || 'object'} ${id}`,
 }))
 
 export const unauthorized = create(2, () => ({
diff --git a/packages/xo-server/src/xo-mixins/backups-ng-logs.js b/packages/xo-server/src/xo-mixins/backups-ng-logs.js
index a3044aa4a..c5dd1c58f 100644
--- a/packages/xo-server/src/xo-mixins/backups-ng-logs.js
+++ b/packages/xo-server/src/xo-mixins/backups-ng-logs.js
@@ -1,8 +1,9 @@
 import { forEach } from 'lodash'
+import { noSuchObject } from 'xo-common/api-errors'
 
 const isSkippedError = error =>
   error.message === 'no disks found' ||
-  error.message === 'no such object' ||
+  noSuchObject.is(error) ||
   error.message === 'no VMs match this pattern' ||
   error.message === 'unhealthy VDI chain'
 
diff --git a/packages/xo-server/src/xo-mixins/backups-ng/index.js b/packages/xo-server/src/xo-mixins/backups-ng/index.js
index 386c045fd..df7f90a8b 100644
--- a/packages/xo-server/src/xo-mixins/backups-ng/index.js
+++ b/packages/xo-server/src/xo-mixins/backups-ng/index.js
@@ -410,6 +410,35 @@ const wrapTaskFn = <T>(
     }
   }
 
+const extractIdsFromSimplePattern = (pattern: mixed) => {
+  if (pattern === null || typeof pattern !== 'object') {
+    return
+  }
+
+  let keys = Object.keys(pattern)
+  if (keys.length !== 1 || keys[0] !== 'id') {
+    return
+  }
+
+  pattern = pattern.id
+  if (typeof pattern === 'string') {
+    return [pattern]
+  }
+  if (pattern === null || typeof pattern !== 'object') {
+    return
+  }
+
+  keys = Object.keys(pattern)
+  if (
+    keys.length === 1 &&
+    keys[0] === '__or' &&
+    Array.isArray((pattern = pattern.__or)) &&
+    pattern.every(_ => typeof _ === 'string')
+  ) {
+    return pattern
+  }
+}
+
 // File structure on remotes:
 //
 // <remote>
@@ -471,21 +500,48 @@ export default class BackupNg {
         }
 
         const job: BackupJob = (job_: any)
+        const vmsPattern = job.vms
 
-        const vms: $Dict<Vm> = app.getObjects({
-          filter: createPredicate({
-            type: 'VM',
-            ...(vmsId !== undefined
-              ? {
-                  id: {
-                    __or: vmsId,
-                  },
-                }
-              : job.vms),
-          }),
-        })
-        if (isEmpty(vms)) {
-          throw new Error('no VMs match this pattern')
+        let vms: $Dict<Vm>
+        if (
+          vmsId !== undefined ||
+          (vmsId = extractIdsFromSimplePattern(vmsPattern)) !== undefined
+        ) {
+          vms = vmsId
+            .map(id => {
+              try {
+                return app.getObject(id, 'VM')
+              } catch (error) {
+                const taskId: string = logger.notice(
+                  `Starting backup of ${id}. (${job.id})`,
+                  {
+                    event: 'task.start',
+                    parentId: runJobId,
+                    data: {
+                      type: 'VM',
+                      id,
+                    },
+                  }
+                )
+                logger.error(`Backuping ${id} has failed. (${job.id})`, {
+                  event: 'task.end',
+                  taskId,
+                  status: 'failure',
+                  result: serializeError(error),
+                })
+              }
+            })
+            .filter(vm => vm !== undefined)
+        } else {
+          vms = app.getObjects({
+            filter: createPredicate({
+              type: 'VM',
+              ...vmsPattern,
+            }),
+          })
+          if (isEmpty(vms)) {
+            throw new Error('no VMs match this pattern')
+          }
         }
         const jobId = job.id
         const srs = unboxIds(job.srs).map(id => {