Created
November 2, 2017 01:37
-
-
Save misterdjules/095d9fc324a1e606c841482db7270bf9 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/lib/errors.js b/lib/errors.js | |
index 825c42f..5f3f6a1 100644 | |
--- a/lib/errors.js | |
+++ b/lib/errors.js | |
@@ -342,6 +342,18 @@ DefaultFabricNetworkNotConfiguredError.prototype.restCode = | |
'DefaultFabricNetworkNotConfiguredError'; | |
DefaultFabricNetworkNotConfiguredError.prototype.statusCode = 409; | |
+function isDataVersionError(err) { | |
+ assert.object(err, 'err'); | |
+ | |
+ return err.name === 'DataVersionError'; | |
+} | |
+ | |
+function isInternalMetadataSearchError(err) { | |
+ assert.object(err, 'err'); | |
+ | |
+ return err.name === 'InternalServerError' && | |
+ err.message.indexOf('internal_metadata') !== -1; | |
+} | |
// ---- exports | |
module.exports = { | |
@@ -357,6 +369,10 @@ module.exports = { | |
// Internal SDC API wrappers | |
vmapiErrorWrap: vmapiErrorWrap, | |
- volapiErrorWrap: volapiErrorWrap | |
+ volapiErrorWrap: volapiErrorWrap, | |
+ | |
+ // Utility functions | |
+ isDataVersionError: isDataVersionError, | |
+ isInternalMetadataSearchError: isInternalMetadataSearchError | |
}; | |
// vim: set softtabstop=4 shiftwidth=4: | |
diff --git a/lib/machines.js b/lib/machines.js | |
index 71fb7e6..1c5e2f9 100644 | |
--- a/lib/machines.js | |
+++ b/lib/machines.js | |
@@ -20,6 +20,7 @@ var libuuid = require('libuuid'); | |
var semver = require('semver'); | |
var clone = require('clone'); | |
var vasync = require('vasync'); | |
+var jsprim = require('jsprim'); | |
var errors = require('./errors'); | |
var images = require('./datasets'); | |
@@ -857,8 +858,9 @@ function loadMachine(req, res, next) { | |
* github.com/joyent/rfd/blob/master/rfd/0026/README.md for | |
* more details. | |
*/ | |
- if (m && m.tags && | |
- m.tags['smartdc_role'] === 'nfsvolumestorage') { | |
+ if (m && m.internal_metadata && | |
+ m.internal_metadata['sdc:system_role'] === | |
+ 'nfsvolumestorage') { | |
cb(new ResourceNotFoundError('VM not found')); | |
return; | |
} | |
@@ -1550,10 +1552,13 @@ function list(req, res, next) { | |
if (req.accountMgmt) { | |
resources.getRoleTags(req, res); | |
} | |
+ | |
+ var listVmsPredicate; | |
var customer = req.account.uuid; | |
var log = req.log; | |
- var volumeStorageVmJsonFilter = {ne: ['tags', | |
- '*smartdc_role=nfsvolumestorage*']}; | |
+ var originalPredicate; | |
+ var volumeStorageVmJsonFilter = {ne: ['internal_metadata.sdc:system_role', | |
+ 'nfsvolumestorage']}; | |
try { | |
var opts = getListOptions(req); | |
@@ -1565,18 +1570,22 @@ function list(req, res, next) { | |
} | |
} | |
+ if (opts) { | |
+ originalPredicate = opts.predicate; | |
+ } | |
+ | |
/* | |
* NFS volumes' storage VMs are an implementation detail that shouldn't be | |
* exposed to end users, so we filter them out. See RFD 26 at | |
* https://github.com/joyent/rfd/blob/master/rfd/0026/README.md for more | |
* details. | |
*/ | |
- if (opts && opts.predicate !== undefined) { | |
- opts.predicate = JSON.stringify({ and: [ | |
- JSON.parse(opts.predicate), volumeStorageVmJsonFilter | |
+ if (originalPredicate !== undefined) { | |
+ listVmsPredicate = JSON.stringify({ and: [ | |
+ JSON.parse(originalPredicate), volumeStorageVmJsonFilter | |
]}); | |
} else { | |
- opts.predicate = JSON.stringify(volumeStorageVmJsonFilter); | |
+ listVmsPredicate = JSON.stringify(volumeStorageVmJsonFilter); | |
} | |
opts.owner_uuid = customer; | |
@@ -1592,38 +1601,70 @@ function list(req, res, next) { | |
loadNetworkUuids(req, machine, cb); | |
} | |
- return req.sdc.vmapi.listVms(opts, { | |
- log: req.log, | |
- headers: { | |
- 'x-request-id': req.getId() | |
- } | |
- }, function (err, machines) { | |
- if (err) { | |
- return next(err); | |
+ function doList() { | |
+ var listVmsOpts = jsprim.deepCopy(opts); | |
+ | |
+ if (listVmsPredicate) { | |
+ listVmsOpts.predicate = listVmsPredicate; | |
} | |
- return vasync.forEachPipeline({ | |
- inputs: machines, | |
- func: addNetworks | |
- }, function (err2) { | |
- if (err2) { | |
- return next(err2); | |
+ return req.sdc.vmapi.listVms(listVmsOpts, { | |
+ log: req.log, | |
+ headers: { | |
+ 'x-request-id': req.getId() | |
} | |
+ }, function (err, machines) { | |
+ if (err) { | |
+ /* | |
+ * If we get a DataVersion error back, or an Internal Error | |
+ * whose message communicate that we can't search on | |
+ * internal_metadata, then it's acceptable to give up on | |
+ * filtering out storage VMs from the ListMachines endpoint. | |
+ * This should be temporary while VMAPI data migrations are in | |
+ * progress, and exposing those machines does not expose any | |
+ * sensible data. | |
+ */ | |
+ if (errors.isDataVersionError(err) || | |
+ errors.isInternalMetadataSearchError(err)) { | |
+ listVmsPredicate = originalPredicate; | |
+ req.log.info({ | |
+ err: err, | |
+ predicateUsed: listVmsOpts.predicate, | |
+ nextPredicateToUse: listVmsPredicate | |
+ }, 'Got error due to filtering on internal_metadata ' + | |
+ 'not supported, retrying without internal_metadata ' + | |
+ 'in search predicate'); | |
+ return doList(); | |
+ } | |
- // NB: machines was mutated by the addNetworks() calls | |
- var translated = machines.map(function (m) { | |
- return translate(m, req); | |
- }); | |
+ return next(err); | |
+ } | |
+ | |
+ return vasync.forEachPipeline({ | |
+ inputs: machines, | |
+ func: addNetworks | |
+ }, function (err2) { | |
+ if (err2) { | |
+ return next(err2); | |
+ } | |
- log.debug('ListMachines(%s) => %j', customer, translated); | |
+ // NB: machines was mutated by the addNetworks() calls | |
+ var translated = machines.map(function (m) { | |
+ return translate(m, req); | |
+ }); | |
- res.header('x-query-limit', opts.limit); | |
- res.header('x-resource-count', translated.length); | |
- res.send(translated); | |
+ log.debug('ListMachines(%s) => %j', customer, translated); | |
- return next(); | |
+ res.header('x-query-limit', opts.limit); | |
+ res.header('x-resource-count', translated.length); | |
+ res.send(translated); | |
+ | |
+ return next(); | |
+ }); | |
}); | |
- }); | |
+ } | |
+ | |
+ return doList(); | |
} | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment