From c152fa77e41ff1314712aa50d83128d0a08ebaab Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 20 Mar 2017 18:55:59 -0700 Subject: [PATCH] more changes according to first code review --- CHANGES.md | 14 ++++++--- lib/cloudapi2.js | 38 ++++++++++++++---------- lib/common.js | 24 +++++++-------- lib/do_volume/do_create.js | 40 ++++++++++++++++++++----- lib/do_volume/do_delete.js | 39 ++++++++++++------------- lib/do_volume/do_list.js | 17 +++++++---- lib/do_volumes.js | 8 +++-- lib/volumes.js | 60 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 170 insertions(+), 70 deletions(-) create mode 100644 lib/volumes.js diff --git a/CHANGES.md b/CHANGES.md index 52b4644..a217b05 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,10 +9,16 @@ Known issues: ## 5.2.0 -- [joyent/mode-triton#173] Add support for listing and getting triton nfs - volumes. -- [joyent/mode-triton#174] Add support for creating triton nfs volumes. -- [joyent/mode-triton#175] Add support for deleting triton nfs volumes. +- Add support for creating and managing NFS shared volumes. New `triton volume` + commands are available: + + * `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. diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index b546b25..9a09012 100644 --- a/lib/cloudapi2.js +++ b/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) { - cb(err, body, res); - }); + this._passThrough(endpoint, cb); }; /** @@ -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 - * to be created + * - size {Number} Optional: a number representing the size of the volume + * 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) { - callback(null, volume, res); + if (opts.states.indexOf(vol.state) !== -1) { + callback(null, vol, res); return; } setTimeout(poll, interval); diff --git a/lib/common.js b/lib/common.js index 56bd521..29d33b7 100644 --- a/lib/common.js +++ b/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) { - if (typeof ('compositionType') === 'undefined') { - compositionType = valid; - valid = undefined; - } - +function jsonPredFromKv(kvs, validKeys, compositionType) { 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: diff --git a/lib/do_volume/do_create.js b/lib/do_volume/do_create.js index de6a630..6f593f9 100644 --- a/lib/do_volume/do_create.js +++ b/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, callback) { +function do_create(subcmd, opts, args, cb) { 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: '', - completionType: 'volumesize' + help: 'The `size` input parameter must match the following regular ' + + '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'], diff --git a/lib/do_volume/do_delete.js b/lib/do_volume/do_delete.js index 0316639..9758e23 100644 --- a/lib/do_volume/do_delete.js +++ b/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); - callback(err); + cb(err); return; } console.log('%s volume %s', common.capitalize('delete'), args); - callback(); + cb(); }); } diff --git a/lib/do_volume/do_list.js b/lib/do_volume/do_list.js index cde1e09..65d0e99 100644 --- a/lib/do_volume/do_list.js +++ b/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 */ diff --git a/lib/do_volumes.js b/lib/do_volumes.js index e3a36bc..0cf9fa3 100644 --- a/lib/do_volumes.js +++ b/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; diff --git a/lib/volumes.js b/lib/volumes.js new file mode 100644 index 0000000..d839aac --- /dev/null +++ b/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 +}; \ No newline at end of file