more changes according to first code review

CHANGES.md

@@ -9,10 +9,16 @@ Known issues:
 
 ## 5.2.0
 
-- [joyent/mode-triton#173] Add support for listing and getting triton nfs
+- Add support for creating and managing NFS shared volumes. New `triton volume`
-  volumes.
+  commands are available:
-- [joyent/mode-triton#174] Add support for creating triton nfs volumes.
+
-- [joyent/mode-triton#175] Add support for deleting triton nfs volumes.
+  * `triton volume create` to create NFS shared volumes
+  * `triton volume list` to list existing volumes
+  * `triton volume get` to get information about a given volume
+  * `triton volume delete` to delete one or more volumes
+
+  Use `triton volume --help` to get help on all of these commands.
+
 - [joyent/node-triton#183] `triton profile create` will no longer use ANSI
   codes for styling if stdout isn't a TTY.
 

lib/cloudapi2.js

@@ -2287,9 +2287,7 @@ CloudApi.prototype.getVolume = function getVolume(opts, cb) {
     assert.uuid(opts.id, 'opts.id');
 
     var endpoint = format('/%s/volumes/%s', this.account, opts.id);
-    this._request(endpoint, function (err, req, res, body) {
+    this._passThrough(endpoint, cb);
-        cb(err, body, res);
-    });
 };
 
 /**
@@ -2308,25 +2306,33 @@ CloudApi.prototype.listVolumes = function listVolumes(options, cb) {
  *
  * @param {Object} options
  *      - name {String} Optional: the name of the volume to be created
- *      - size {String} Optional: a string representing the size of the volume
+ *      - size {Number} Optional: a number representing the size of the volume
- *        to be created
+ *        to be created in mebibytes.
  *      - networks {Array} Optional: an array that contains all the networks
  *        that should be reachable from the newly created volume
+ *      - type {String}: the type of the volume. Currently, only "tritonnfs" is
+ *        supported.
  * @param {Function} callback - called like `function (err, volume, res)`
  */
-CloudApi.prototype.createVolume = function createVolume(options, callback) {
+CloudApi.prototype.createVolume = function createVolume(options, cb) {
     assert.object(options, 'options');
     assert.optionalString(options.name, 'options.name');
-    assert.optionalString(options.size, 'options.size');
+    assert.optionalNumber(options.size, 'options.size');
     assert.optionalArrayOfUuid(options.networks, 'options.networks');
-    assert.func(callback, 'callback');
+    assert.string(options.type, 'options.type');
+    assert.func(cb, 'cb');
 
     this._request({
         method: 'POST',
         path: format('/%s/volumes', this.account),
-        data: options
+        data: {
+            name: options.name,
+            size: options.size,
+            networks: options.networks,
+            type: options.type
+        }
     }, function (err, req, res, body) {
-        callback(err, body, res);
+        cb(err, body, res);
     });
 };
 
@@ -2336,15 +2342,15 @@ CloudApi.prototype.createVolume = function createVolume(options, callback) {
  * @param {String} volumeUuid
  * @param {Function} callback - called like `function (err, volume, res)`
  */
-CloudApi.prototype.deleteVolume = function deleteVolume(volumeUuid, callback) {
+CloudApi.prototype.deleteVolume = function deleteVolume(volumeUuid, cb) {
     assert.uuid(volumeUuid, 'volumeUuid');
-    assert.func(callback, 'callback');
+    assert.func(cb, 'cb');
 
     this._request({
         method: 'DELETE',
         path: format('/%s/volumes/%s', this.account, volumeUuid)
     }, function (err, req, res, body) {
-        callback(err, body, res);
+        cb(err, res);
     });
 };
 
@@ -2374,13 +2380,13 @@ function waitForVolumeStates(opts, callback) {
     function poll() {
         self.getVolume({
             id: opts.id
-        }, function (err, volume, res) {
+        }, function (err, vol, res) {
             if (err) {
                 callback(err, null, res);
                 return;
             }
-            if (opts.states.indexOf(volume.state) !== -1) {
+            if (opts.states.indexOf(vol.state) !== -1) {
-                callback(null, volume, res);
+                callback(null, vol, res);
                 return;
             }
             setTimeout(poll, interval);

lib/common.js

@@ -160,19 +160,15 @@ function kvToObj(kvs, valid) {
  * given an array of key=value pairs, break them into a JSON predicate
  *
  * @param {Array} kvs - an array of key=value pairs
- * @param {Array} valid (optional) - an array to validate pairs
+ * @param {Array} validKeys (optional) - an array representing valid keys that
+ *  can be used in the first argument "kvs".
  * @param {String} compositionType - the way each key/value pair will be
- * combined to form a JSON predicate. Valid values are 'or' and 'and'
+ *  combined to form a JSON predicate. Valid values are 'or' and 'and'.
  *
  */
-function kvToJSONPredicate(kvs, valid, compositionType) {
+function jsonPredFromKv(kvs, validKeys, compositionType) {
-    if (typeof ('compositionType') === 'undefined') {
-        compositionType = valid;
-        valid = undefined;
-    }
-
     assert.arrayOfString(kvs, 'kvs');
-    assert.optionalArrayOfString(valid, 'valid');
+    assert.arrayOfString(validKeys, 'validKeys');
     assert.string(compositionType, 'string');
     assert.ok(compositionType === 'or' || compositionType === 'and',
         'compositionType');
@@ -183,16 +179,18 @@ function kvToJSONPredicate(kvs, valid, compositionType) {
     for (var i = 0; i < kvs.length; i++) {
         var kv = kvs[i];
         var idx = kv.indexOf('=');
-        if (idx === -1)
+        if (idx === -1) {
              throw new errors.UsageError(format(
                 'invalid filter: "%s" (must be of the form "field=value")',
                 kv));
+        }
         var k = kv.slice(0, idx);
         var v = kv.slice(idx + 1);
-        if (valid && valid.indexOf(k) === -1)
+        if (validKeys && validKeys.indexOf(k) === -1) {
              throw new errors.UsageError(format(
                 'invalid filter name: "%s" (must be one of "%s")',
-                k, valid.join('", "')));
+                k, validKeys.join('", "')));
+        }
         predicate[compositionType].push({eq: [k, v]});
     }
 
@@ -1159,6 +1157,6 @@ module.exports = {
     deepEqual: deepEqual,
     tildeSync: tildeSync,
     objFromKeyValueArgs: objFromKeyValueArgs,
-    kvToJSONPredicate: kvToJSONPredicate
+    jsonPredFromKv: jsonPredFromKv
 };
 // vim: set softtabstop=4 shiftwidth=4:

lib/do_volume/do_create.js

@@ -17,19 +17,39 @@ var vasync = require('vasync');
 var common = require('../common');
 var distractions = require('../distractions');
 var errors = require('../errors');
+var mod_volumes = require('../volumes');
 
-
+function do_create(subcmd, opts, args, cb) {
-function do_create(subcmd, opts, args, callback) {
     var self = this;
 
     if (opts.help) {
-        this.do_help('help', {}, [subcmd], callback);
+        this.do_help('help', {}, [subcmd], cb);
+        return;
+    }
+
+    if (args.length !== 0) {
+        cb(new errors.UsageError('incorrect number of args'));
         return;
     }
 
     var context = {};
 
     vasync.pipeline({funcs: [
+        function validateVolumeSize(ctx, next) {
+            if (opts.size === undefined) {
+                next();
+                return;
+            }
+
+            try {
+                ctx.size = mod_volumes.parseVolumeSize(opts.size);
+            } catch (parseSizeErr) {
+                next(parseSizeErr);
+                return;
+            }
+
+            next();
+        },
         function setup(ctx, next) {
             common.cliSetupTritonApi({
                 cli: self.top
@@ -63,7 +83,7 @@ function do_create(subcmd, opts, args, callback) {
                 type: 'tritonnfs',
                 name: opts.name,
                 networks: ctx.networks,
-                size: opts.size
+                size: ctx.size
             };
 
             if (opts.type) {
@@ -102,7 +122,7 @@ function do_create(subcmd, opts, args, callback) {
                 console.log(JSON.stringify(ctx.volume, null, 4));
             }
         }
-    ], arg: context}, callback);
+    ], arg: context}, cb);
 }
 
 do_create.options = [
@@ -127,11 +147,15 @@ do_create.options = [
         help: 'Volume type. Default is "tritonnfs".'
     },
     {
-        names: ['size', 'S'],
+        names: ['size', 's'],
         type: 'string',
         helpArg: 'SIZE',
-        help: '',
+        help: 'The `size` input parameter must match the following regular ' +
-        completionType: 'volumesize'
+            'expression: /(\d+)(g|m|G|M|gb|mb|GB|MB)/ All units are in ' +
+            'ibibytes (mebibytes and gibibytes). `g`, `G`, `gb` and `GB` ' +
+            'stand for "gibibytes". `m`, `M`, `mb` and `MB` stand for ' +
+            '"mebibytes".',
+        completionType: 'tritonvolumesize'
     },
     {
         names: ['network', 'N'],

lib/do_volume/do_delete.js

@@ -7,7 +7,7 @@
 /*
  * Copyright 2017 Joyent, Inc.
  *
- * `triton volume del ...`
+ * `triton volume delete ...`
  */
 
 var assert = require('assert-plus');
@@ -18,16 +18,14 @@ var common = require('../common');
 var distractions = require('../distractions');
 var errors = require('../errors');
 
-function perror(err) {
-    console.error('error: %s', err.message);
-}
-function deleteVolume(volumeName, options, callback) {
-    assert.string(volumeName, 'volumeName');
-    assert.object(options, 'options');
-    assert.object(options.tritonapi, 'options.tritonapi');
-    assert.func(callback, 'callback');
 
-    var tritonapi = options.tritonapi;
+function deleteVolume(volumeName, opts, cb) {
+    assert.string(volumeName, 'volumeName');
+    assert.object(opts, 'opts');
+    assert.object(opts.tritonapi, 'opts.tritonapi');
+    assert.func(cb, 'cb');
+
+    var tritonapi = opts.tritonapi;
 
     vasync.pipeline({funcs: [
         function getVolume(ctx, next) {
@@ -52,12 +50,12 @@ function deleteVolume(volumeName, options, callback) {
             var distraction;
             var volumeId = ctx.volume.id;
 
-            if (!options.wait) {
+            if (!opts.wait) {
                 next();
                 return;
             }
 
-            distraction = distractions.createDistraction(options.wait.length);
+            distraction = distractions.createDistraction(opts.wait.length);
 
             tritonapi.cloudapi.waitForVolumeStates({
                 id: volumeId,
@@ -67,17 +65,17 @@ function deleteVolume(volumeName, options, callback) {
                 next(waitErr);
             });
         }
-    ], arg: {}}, callback);
+    ], arg: {}}, cb);
 }
 
-function do_delete(subcmd, opts, args, callback) {
+function do_delete(subcmd, opts, args, cb) {
     var self = this;
 
     if (opts.help) {
-        self.do_help('help', {}, [subcmd], callback);
+        self.do_help('help', {}, [subcmd], cb);
         return;
     } else if (args.length < 1) {
-        callback(new errors.UsageError('missing VOLUME arg(s)'));
+        cb(new errors.UsageError('missing VOLUME arg(s)'));
         return;
     }
 
@@ -85,7 +83,7 @@ function do_delete(subcmd, opts, args, callback) {
         volumeIds: args
     };
 
-    vasync.pipeline({funcs: [
+    vasync.pipeline({arg: context, funcs: [
         function setup(ctx, next) {
             common.cliSetupTritonApi({
                 cli: self.top
@@ -104,16 +102,15 @@ function do_delete(subcmd, opts, args, callback) {
                 inputs: ctx.volumeIds
             }, next);
         }
-    ], arg: context}, function onDone(err) {
+    ]}, function onDone(err) {
         if (err) {
-            perror(err);
+            cb(err);
-            callback(err);
             return;
         }
 
         console.log('%s volume %s', common.capitalize('delete'), args);
 
-        callback();
+        cb();
     });
 }
 

lib/do_volume/do_list.js

@@ -7,7 +7,7 @@
 /*
  * Copyright 2016 Joyent, Inc.
  *
- * `triton image list ...`
+ * `triton volume list ...`
  */
 
 var format = require('util').format;
@@ -25,13 +25,13 @@ var validFilters = [
 ];
 
 // columns default without -o
-var columnsDefault = 'shortid,name,size,type,state';
+var columnsDefault = 'shortid,name,size,type,state,age';
 
 // columns default with -l
-var columnsDefaultLong = 'id,name,size,type,state';
+var columnsDefaultLong = 'id,name,size,type,state,created';
 
 // sort default with -s
-var sortDefault = 'create_timestamp';
+var sortDefault = 'created';
 
 function do_list(subcmd, opts, args, callback) {
     var self = this;
@@ -56,7 +56,7 @@ function do_list(subcmd, opts, args, callback) {
 
     if (args) {
         try {
-            filterPredicate = common.kvToJSONPredicate(args, validFilters,
+            filterPredicate = common.jsonPredFromKv(args, validFilters,
                 'and');
         } catch (e) {
             callback(e);
@@ -85,6 +85,8 @@ function do_list(subcmd, opts, args, callback) {
         }
         self.top.tritonapi.cloudapi.listVolumes(listOpts,
             function onRes(listVolsErr, volumes, res) {
+                var now;
+
                 if (listVolsErr) {
                     return callback(listVolsErr);
                 }
@@ -92,9 +94,12 @@ function do_list(subcmd, opts, args, callback) {
                 if (opts.json) {
                     common.jsonStream(volumes);
                 } else {
+                    now = new Date();
                     for (var i = 0; i < volumes.length; i++) {
                         var volume = volumes[i];
                         volume.shortid = volume.id.split('-', 1)[0];
+                        volume.created = new Date(volume.create_timestamp);
+                        volume.age = common.longAgo(volume.created, now);
                     }
 
                     tabula(volumes, {
@@ -119,7 +124,7 @@ do_list.options = [
     sortDefault: sortDefault
 }));
 
-do_list.synopses = ['{{name}} {{cmd}} [OPTIONS]'];
+do_list.synopses = ['{{name}} {{cmd}} [OPTIONS] [FILTERS]'];
 
 do_list.help = [
     /* BEGIN JSSTYLED */

lib/do_volumes.js

@@ -10,6 +10,8 @@
  * `triton volumes ...` bwcompat shortcut for `triton volumes list ...`.
  */
 
+var targ = require('./do_volume/do_list');
+
 function do_volumes(subcmd, opts, args, callback) {
     this.handlerFromSubcmd('volume').dispatch({
         subcmd: 'list',
@@ -18,9 +20,11 @@ function do_volumes(subcmd, opts, args, callback) {
     }, callback);
 }
 
-do_volumes.help = 'A shortcut for "triton volumes list".';
+do_volumes.help = 'A shortcut for "triton volumes list".\n' + targ.help;
 do_volumes.aliases = ['vols'];
 do_volumes.hidden = true;
-do_volumes.options = require('./do_volume/do_list').options;
+do_volumes.options = targ.options;
+do_volumes.completionArgtypes = targ.completionArgtypes;
+do_volumes.synopses = targ.synopses;
 
 module.exports = do_volumes;

lib/volumes.js

@@ -0,0 +1,60 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+/*
+ * Copyright (c) 2017, Joyent, Inc.
+ */
+
+var assert = require('assert-plus');
+
+function throwInvalidSize(size) {
+    assert.string(size, 'size');
+
+    throw new Error('size "' + size + '" is not a valid volume size');
+}
+
+/*
+ * Returns the number of MiBs (Mebibytes) represented by the string "size". That
+ * string can have different format suffixes, such as "100GB", "100G", etc.
+ * If "size" is not a valid size string, an error is thrown.
+ */
+function parseVolumeSize(size) {
+    assert.optionalString(size, 'size');
+
+    var MIBS_IN_GB = 1024;
+
+    var MULTIPLIERS_TABLE = {
+        g: MIBS_IN_GB,
+        GB: MIBS_IN_GB,
+        m: 1,
+        MB: 1
+    };
+
+    var multiplierSymbol, multiplier;
+    var baseValue;
+
+    if (size === undefined) {
+        return undefined;
+    }
+
+    var matches = size.match(/(\d+)(g|m|G|M|gb|mb|GB|MB)/);
+    if (!matches) {
+        throwInvalidSize(size);
+    }
+
+    multiplierSymbol = matches[2];
+    multiplier = MULTIPLIERS_TABLE[multiplierSymbol];
+    baseValue = Number(matches[1]);
+    if (isNaN(baseValue) || multiplier === undefined) {
+        throwInvalidSize(size);
+    }
+
+    return baseValue * multiplier;
+}
+
+module.exports = {
+    parseVolumeSize: parseVolumeSize
+};