From 00f92d67a5f9ebc5383cc8f632333126a16bcfa0 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Wed, 22 Mar 2017 16:54:36 -0700 Subject: [PATCH] more changes according to discussion after initial code review --- lib/cloudapi2.js | 30 +++++++++++++- lib/common.js | 22 +++++++++- lib/do_volume/do_create.js | 35 +++++++++------- lib/do_volume/do_delete.js | 76 ++++++++++++++++++++++++++++------ lib/tritonapi.js | 83 +++++++++++++++++++++++++++++++++++--- 5 files changed, 212 insertions(+), 34 deletions(-) diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index 9a09012..c147c57 100644 --- a/lib/cloudapi2.js +++ b/lib/cloudapi2.js @@ -2362,6 +2362,9 @@ CloudApi.prototype.deleteVolume = function deleteVolume(volumeUuid, cb) { * - {String} id - machine UUID * - {Array of String} states - desired state * - {Number} interval (optional) - time in ms to poll + * - {Number} timeout (optional) - time in ms after which "callback" is + * called with an error object if the volume hasn't yet transitioned to + * one of the states in "opts.states". * @param {Function} callback - called when state is reached or on error */ CloudApi.prototype.waitForVolumeStates = @@ -2371,16 +2374,25 @@ function waitForVolumeStates(opts, callback) { assert.uuid(opts.id, 'opts.id'); assert.arrayOfString(opts.states, 'opts.states'); assert.optionalNumber(opts.interval, 'opts.interval'); + assert.optionalNumber(opts.timeout, 'opts.timeout'); assert.func(callback, 'callback'); + var interval = (opts.interval === undefined ? 1000 : opts.interval); + interval = Math.min(interval, opts.timeout); assert.ok(interval > 0, 'interval must be a positive number'); + var startTime = process.hrtime(); + var timeout = opts.timeout; + poll(); function poll() { self.getVolume({ id: opts.id }, function (err, vol, res) { + var elapsedTime; + var timedOut = false; + if (err) { callback(err, null, res); return; @@ -2388,8 +2400,24 @@ function waitForVolumeStates(opts, callback) { if (opts.states.indexOf(vol.state) !== -1) { callback(null, vol, res); return; + } else { + if (timeout !== undefined) { + elapsedTime = common.monotonicTimeDiffMs(startTime); + if (elapsedTime > timeout) { + timedOut = true; + } + } + + if (timedOut) { + callback(new errors.TimeoutError(format('timeout waiting ' + + 'for state changes on volume %s (elapsed %ds)', + opts.id, Math.round(elapsedTime / 1000)))); + return; + } else { + setTimeout(poll, interval); + return; + } } - setTimeout(poll, interval); }); } }; diff --git a/lib/common.js b/lib/common.js index 29d33b7..d85f7cd 100644 --- a/lib/common.js +++ b/lib/common.js @@ -1120,6 +1120,25 @@ function objFromKeyValueArgs(args, opts) return obj; } +/** + * Returns the time difference between the current time and the time + * represented by "relativeTo" in milliseconds. It doesn't use the built-in + * `Date` class internally, and instead uses a node facility that uses a + * monotonic clock. Thus, the time difference computed is not subject to time + * drifting due to e.g changes in the wall clock system time. + * + * @param {arrayOfNumber} relativeTo: an array representing the starting time as + * returned by `process.hrtime()` from which to compute the + * time difference. + */ +function monotonicTimeDiffMs(relativeTo) { + assert.arrayOfNumber(relativeTo, 'relativeTo'); + + var diff = process.hrtime(relativeTo); + var ms = (diff[0] * 1e3) + (diff[1] / 1e6); // in milliseconds + return ms; +} + //---- exports @@ -1157,6 +1176,7 @@ module.exports = { deepEqual: deepEqual, tildeSync: tildeSync, objFromKeyValueArgs: objFromKeyValueArgs, - jsonPredFromKv: jsonPredFromKv + jsonPredFromKv: jsonPredFromKv, + monotonicTimeDiffMs: monotonicTimeDiffMs }; // vim: set softtabstop=4 shiftwidth=4: diff --git a/lib/do_volume/do_create.js b/lib/do_volume/do_create.js index 6f593f9..90aed80 100644 --- a/lib/do_volume/do_create.js +++ b/lib/do_volume/do_create.js @@ -32,9 +32,7 @@ function do_create(subcmd, opts, args, cb) { return; } - var context = {}; - - vasync.pipeline({funcs: [ + vasync.pipeline({arg: {cli: this.top}, funcs: [ function validateVolumeSize(ctx, next) { if (opts.size === undefined) { next(); @@ -50,13 +48,7 @@ function do_create(subcmd, opts, args, cb) { next(); }, - function setup(ctx, next) { - common.cliSetupTritonApi({ - cli: self.top - }, function onSetup(setupErr) { - next(setupErr); - }); - }, + common.cliSetupTritonApi, function getNetworks(ctx, next) { if (!opts.network) { return next(); @@ -97,18 +89,26 @@ function do_create(subcmd, opts, args, cb) { }); }, function maybeWait(ctx, next) { + var distraction; + if (!opts.wait) { next(); return; } - var distraction = distractions.createDistraction(opts.wait.length); + if (process.stderr.isTTY && opts.wait.length > 1) { + distraction = distractions.createDistraction(opts.wait.length); + } self.top.tritonapi.cloudapi.waitForVolumeStates({ id: ctx.volume.id, - states: ['ready', 'failed'] + states: ['ready', 'failed'], + timeout: opts.wait_timeout * 1000 }, function onWaitDone(waitErr, volume) { - distraction.destroy(); + if (distraction) { + distraction.destroy(); + } + ctx.volume = volume; next(waitErr); }); @@ -122,7 +122,7 @@ function do_create(subcmd, opts, args, cb) { console.log(JSON.stringify(ctx.volume, null, 4)); } } - ], arg: context}, cb); + ]}, cb); } do_create.options = [ @@ -178,6 +178,13 @@ do_create.options = [ type: 'arrayOfBool', help: 'Wait for the creation to complete. Use multiple times for a ' + 'spinner.' + }, + { + names: ['wait-timeout'], + type: 'positiveInteger', + default: 120, + help: 'The number of seconds to wait before timing out with an error. ' + + 'The default is 120 seconds.' } ]; diff --git a/lib/do_volume/do_delete.js b/lib/do_volume/do_delete.js index 9758e23..f244e94 100644 --- a/lib/do_volume/do_delete.js +++ b/lib/do_volume/do_delete.js @@ -80,36 +80,76 @@ function do_delete(subcmd, opts, args, cb) { } var context = { - volumeIds: args + volumeIds: args, + cli: this.top }; vasync.pipeline({arg: context, funcs: [ - function setup(ctx, next) { - common.cliSetupTritonApi({ - cli: self.top - }, function onSetup(setupErr) { - next(setupErr); - }); + common.cliSetupTritonApi, + function confirm(ctx, next) { + var promptMsg; + + if (opts.yes) { + next(); + return; + } + + if (ctx.volumeIds.length === 1) { + promptMsg = format('Delete volume %s? [y/n] ', + ctx.volumeIds[0]); + } else { + promptMsg = format('Delete %d volumes? [y/n] ', + ctx.volumeIds.length); + } + + common.promptYesNo({msg: promptMsg}, + function onPromptAnswered(answer) { + if (answer !== 'y') { + console.error('Aborting'); + /* + * Early abort signal. + */ + next(true); + } else { + next(); + } + }); }, function deleteVolumes(ctx, next) { vasync.forEachParallel({ func: function doDeleteVolume(volumeId, done) { - deleteVolume(volumeId, { - wait: opts.wait, + self.top.tritonapi.deleteVolume({ + id: volumeId, + wait: opts.wait && opts.wait.length > 0, + waitTimeout: opts.wait_timeout * 1000, tritonapi: self.top.tritonapi - }, done); + }, function onVolDeleted(volDelErr) { + if (volDelErr) { + console.error('Error when deleting volume %s, ' + + 'reason: %s', volumeId, volDelErr); + } else { + console.log('Deleted volume %s', volumeId); + } + + done(); + }); }, inputs: ctx.volumeIds }, next); } ]}, function onDone(err) { + if (err === true) { + /* + * Answered 'no' to confirmation to delete. + */ + err = null; + } + if (err) { cb(err); return; } - console.log('%s volume %s', common.capitalize('delete'), args); - cb(); }); } @@ -128,6 +168,18 @@ do_delete.options = [ type: 'arrayOfBool', help: 'Wait for the deletion to complete. Use multiple times for a ' + 'spinner.' + }, + { + names: ['wait-timeout'], + type: 'positiveInteger', + default: 120, + help: 'The number of seconds to wait before timing out with an error. ' + + 'The default is 120 seconds.' + }, + { + names: ['yes', 'y'], + type: 'bool', + help: 'Answer yes to confirmation to delete.' } ]; diff --git a/lib/tritonapi.js b/lib/tritonapi.js index 7fd327a..d102fea 100644 --- a/lib/tritonapi.js +++ b/lib/tritonapi.js @@ -2313,11 +2313,6 @@ TritonApi.prototype.rebootInstance = function rebootInstance(opts, cb) { function randrange(min, max) { return Math.floor(Math.random() * (max - min + 1)) + min; } - function timeDiffMs(relativeTo) { - var diff = process.hrtime(relativeTo); - var ms = (diff[0] * 1e3) + (diff[1] / 1e6); // in milliseconds - return ms; - } vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [ _stepInstId, @@ -2398,7 +2393,8 @@ TritonApi.prototype.rebootInstance = function rebootInstance(opts, cb) { if (!theRecord) { if (opts.waitTimeout) { - var elapsedMs = timeDiffMs(startTime); + var elapsedMs = + common.monotonicTimeDiffMs(startTime); if (elapsedMs > opts.waitTimeout) { next(new errors.TimeoutError(format('timeout ' + 'waiting for instance %s reboot ' @@ -2667,6 +2663,81 @@ TritonApi.prototype.getVolume = function getVolume(name, cb) { } }; +/** + * A function appropriate for `vasync.pipeline` funcs that takes a `arg.id` + * volume name, shortid or uuid, and determines the volume id (setting it + * as `arg.volId`). + */ +function _stepVolId(arg, next) { + assert.object(arg.client, 'arg.client'); + assert.string(arg.id, 'arg.id'); + + if (common.isUUID(arg.id)) { + arg.volId = arg.id; + next(); + } else { + arg.client.getVolume(arg.id, function onGetVolume(getVolErr, vol) { + if (getVolErr) { + next(getVolErr); + } else { + arg.volId = vol.id; + next(); + } + }); + } +} + +/** + * Deletes a volume by ID, exact name, or short ID, in that order. + * + * If there is more than one volume with that name, then this errors out. + * + * @param {Object} opts + * - {String} id: Required. The volume to delete's id, name or short ID. + * - {Boolean} wait: Optional. true if "cb" must be called once the volume + * is actually deleted, or deletion failed. If "false", "cb" will be + * called as soon as the deletion process is scheduled. + * - {Number} waitTimeout: Optional. if "wait" is true, represents the + * number of milliseconds after which to timeout (call `cb` with a + * timeout error) waiting. + * @param {Function} cb: `function (err)` + */ +TritonApi.prototype.deleteVolume = function deleteVolume(opts, cb) { + assert.string(opts.id, 'opts.id'); + assert.optionalBool(opts.wait, 'opts.wait'); + assert.optionalNumber(opts.waitTimeout, 'opts.waitTimeout'); + assert.func(cb, 'cb'); + + var self = this; + var res; + + vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [ + _stepVolId, + + function doDelete(arg, next) { + self.cloudapi.deleteVolume(arg.volId, + function onVolDeleted(volDelErr, _, _res) { + res = _res; + next(volDelErr); + }); + }, + + function waitForVolumeDeleted(arg, next) { + if (!opts.wait) { + next(); + return; + } + self.cloudapi.waitForVolumeStates({ + id: arg.volId, + states: ['deleted', 'failed'], + timeout: opts.waitTimeout + }, next); + } + ]}, function onDeletionComplete(err) { + cb(err, null, res); + }); +}; + //---- exports module.exports = {