more changes according to discussion after initial code review

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);
         });
     }
 };

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:

lib/do_volume/do_create.js

@@ -32,9 +32,7 @@ function do_create(subcmd, opts, args, cb) {
         return;
     }
 
-    var context = {};
+    vasync.pipeline({arg: {cli: this.top}, funcs: [
-
-    vasync.pipeline({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,
-            common.cliSetupTritonApi({
-                cli: self.top
-            }, function onSetup(setupErr) {
-                next(setupErr);
-            });
-        },
         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.'
     }
 ];
 

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,
-            common.cliSetupTritonApi({
+        function confirm(ctx, next) {
-                cli: self.top
+            var promptMsg;
-            }, function onSetup(setupErr) {
+
-                next(setupErr);
+            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, {
+                    self.top.tritonapi.deleteVolume({
-                        wait: opts.wait,
+                        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.'
     }
 ];
 

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 = {