From 8f666b7aca435a343c76a4997022fb667d6be55d Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 30 Dec 2015 16:11:54 -0800 Subject: [PATCH] joyent/node-triton#67 `triton create` should support assigning tags Fixes #67. --- CHANGES.md | 5 +- lib/do_create_instance.js | 163 +++++++++++---- test/integration/cli-manage-workflow.test.js | 2 + test/unit/metadataFromOpts.test.js | 9 +- test/unit/tagsFromOpts.test.js | 196 +++++++++++++++++++ 5 files changed, 335 insertions(+), 40 deletions(-) create mode 100644 test/unit/tagsFromOpts.test.js diff --git a/CHANGES.md b/CHANGES.md index d1d5210..5962aa0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,8 +1,9 @@ # node-triton changelog -## 3.4.3 (not yet released) +## 3.5.0 (not yet released) -(nothing yet) +- #67 Add `triton create --tag|-t ...` option for adding tags on instance creation. + E.g. `triton create -n NAME -t foo=bar -t @my-tags-file.json IMAGE PACKAGE`. ## 3.4.2 diff --git a/lib/do_create_instance.js b/lib/do_create_instance.js index d01f722..431c653 100644 --- a/lib/do_create_instance.js +++ b/lib/do_create_instance.js @@ -23,7 +23,7 @@ var distractions = require('./distractions'); var errors = require('./errors'); -// ---- loading/parsing metadata from relevant options +// ---- loading/parsing metadata (and tags) from relevant options /* * Load and validate metadata from these options: @@ -31,7 +31,6 @@ var errors = require('./errors'); * -M,--metadata-file KEY=FILE * --script FILE * - * * * says values may be string, num or bool. */ @@ -52,17 +51,21 @@ function metadataFromOpts(opts, log, cb) { 'empty metadata option value')); return; } else if (o.value[0] === '{') { - _addMetadataFromJsonStr(metadata, o.value, null, next); + _addMetadataFromJsonStr( + 'metadata', metadata, o.value, null, next); } else if (o.value[0] === '@') { - _addMetadataFromFile(metadata, o.value.slice(1), next); + _addMetadataFromFile( + 'metadata', metadata, o.value.slice(1), next); } else { - _addMetadataFromKvStr(metadata, o.value, null, next); + _addMetadataFromKvStr( + 'metadata', metadata, o.value, null, next); } } else if (o.key === 'metadata_file') { - _addMetadataFromKfStr(metadata, o.value, null, next); + _addMetadataFromKfStr( + 'metadata', metadata, o.value, null, next); } else if (o.key === 'script') { - _addMetadatumFromFile(metadata, 'user-script', o.value, - o.value, next); + _addMetadatumFromFile('metadata', metadata, + 'user-script', o.value, o.value, next); } else { next(); } @@ -79,8 +82,55 @@ function metadataFromOpts(opts, log, cb) { } +/* + * Load and validate tags from these options: + * -t,--tag DATA + * + * + * says values may be string, num or bool. + */ +function tagsFromOpts(opts, log, cb) { + assert.arrayOfObject(opts._order, 'opts._order'); + assert.object(log, 'log'); + assert.func(cb, 'cb'); + + var tags = {}; + + vasync.forEachPipeline({ + inputs: opts._order, + func: function tagsFromOpt(o, next) { + log.trace({opt: o}, 'tagsFromOpt'); + if (o.key === 'tag') { + if (!o.value) { + next(new errors.UsageError( + 'empty tag option value')); + return; + } else if (o.value[0] === '{') { + _addMetadataFromJsonStr('tag', tags, o.value, null, next); + } else if (o.value[0] === '@') { + _addMetadataFromFile('tag', tags, o.value.slice(1), next); + } else { + _addMetadataFromKvStr('tag', tags, o.value, null, next); + } + } else { + next(); + } + } + }, function (err) { + if (err) { + cb(err); + } else if (Object.keys(tags).length) { + cb(null, tags); + } else { + cb(); + } + }); +} + + var allowedTypes = ['string', 'number', 'boolean']; -function _addMetadatum(metadata, key, value, from, cb) { +function _addMetadatum(ilk, metadata, key, value, from, cb) { + assert.string(ilk, 'ilk'); assert.object(metadata, 'metadata'); assert.string(key, 'key'); assert.optionalString(from, 'from'); @@ -88,7 +138,8 @@ function _addMetadatum(metadata, key, value, from, cb) { if (allowedTypes.indexOf(typeof (value)) === -1) { cb(new errors.UsageError(format( - 'invalid metadata value type: must be one of %s: %s=%j', + 'invalid %s value type%s: must be one of %s: %s=%j', + ilk, (from ? ' (from ' + from + ')' : ''), allowedTypes.join(', '), key, value))); return; } @@ -96,7 +147,8 @@ function _addMetadatum(metadata, key, value, from, cb) { if (metadata.hasOwnProperty(key)) { var valueStr = value.toString(); console.error( - 'warning: metadata "%s=%s"%s replaces earlier value for "%s"', + 'warning: %s "%s=%s"%s replaces earlier value for "%s"', + ilk, key, (valueStr.length > 10 ? valueStr.slice(0, 7) + '...' : valueStr), @@ -107,7 +159,8 @@ function _addMetadatum(metadata, key, value, from, cb) { cb(); } -function _addMetadataFromObj(metadata, obj, from, cb) { +function _addMetadataFromObj(ilk, metadata, obj, from, cb) { + assert.string(ilk, 'ilk'); assert.object(metadata, 'metadata'); assert.object(obj, 'obj'); assert.optionalString(from, 'from'); @@ -116,24 +169,26 @@ function _addMetadataFromObj(metadata, obj, from, cb) { vasync.forEachPipeline({ inputs: Object.keys(obj), func: function _oneField(key, next) { - _addMetadatum(metadata, key, obj[key], from, next); + _addMetadatum(ilk, metadata, key, obj[key], from, next); } }, cb); } -function _addMetadataFromJsonStr(metadata, s, from, cb) { +function _addMetadataFromJsonStr(ilk, metadata, s, from, cb) { + assert.string(ilk, 'ilk'); try { var obj = JSON.parse(s); } catch (parseErr) { cb(new errors.TritonError(parseErr, - format('metadata%s is not valid JSON', + format('%s%s is not valid JSON', ilk, (from ? ' (from ' + from + ')' : '')))); return; } - _addMetadataFromObj(metadata, obj, from, cb); + _addMetadataFromObj(ilk, metadata, obj, from, cb); } -function _addMetadataFromFile(metadata, file, cb) { +function _addMetadataFromFile(ilk, metadata, file, cb) { + assert.string(ilk, 'ilk'); tilde(file, function (metaPath) { fs.stat(metaPath, function (statErr, stats) { if (statErr || !stats.isFile()) { @@ -153,14 +208,15 @@ function _addMetadataFromFile(metadata, file, cb) { */ var dataTrim = data.trim(); if (dataTrim.length && dataTrim[0] === '{') { - _addMetadataFromJsonStr(metadata, dataTrim, file, cb); + _addMetadataFromJsonStr(ilk, metadata, dataTrim, file, cb); } else { var lines = dataTrim.split(/\r?\n/g).filter( function (line) { return line.trim(); }); vasync.forEachPipeline({ inputs: lines, func: function oneLine(line, next) { - _addMetadataFromKvStr(metadata, line, file, next); + _addMetadataFromKvStr( + ilk, metadata, line, file, next); } }, cb); } @@ -169,11 +225,13 @@ function _addMetadataFromFile(metadata, file, cb) { }); } -function _addMetadataFromKvStr(metadata, s, from, cb) { +function _addMetadataFromKvStr(ilk, metadata, s, from, cb) { + assert.string(ilk, 'ilk'); + var parts = strsplit(s, '=', 2); if (parts.length !== 2) { - cb(new errors.UsageError( - 'invalid KEY=VALUE metadata argument: ' + s)); + cb(new errors.UsageError(format( + 'invalid KEY=VALUE %s argument: %s', ilk, s))); return; } var value = parts[1]; @@ -188,33 +246,36 @@ function _addMetadataFromKvStr(metadata, s, from, cb) { value = num; } } - _addMetadatum(metadata, parts[0].trim(), value, from, cb); + _addMetadatum(ilk, metadata, parts[0].trim(), value, from, cb); } /* * Add metadata from `KEY=FILE` argument. * Here "Kf" stands for "key/file". */ -function _addMetadataFromKfStr(metadata, s, from, cb) { +function _addMetadataFromKfStr(ilk, metadata, s, from, cb) { + assert.string(ilk, 'ilk'); + var parts = strsplit(s, '=', 2); if (parts.length !== 2) { - cb(new errors.UsageError( - 'invalid KEY=FILE metadata argument: ' + s)); + cb(new errors.UsageError(format( + 'invalid KEY=FILE %s argument: %s', ilk, s))); return; } var key = parts[0].trim(); var file = parts[1]; - _addMetadatumFromFile(metadata, key, file, file, cb); + _addMetadatumFromFile(ilk, metadata, key, file, file, cb); } -function _addMetadatumFromFile(metadata, key, file, from, cb) { +function _addMetadatumFromFile(ilk, metadata, key, file, from, cb) { + assert.string(ilk, 'ilk'); + tilde(file, function (filePath) { fs.stat(filePath, function (statErr, stats) { if (statErr || !stats.isFile()) { cb(new errors.TritonError(format( - 'metadata path "%s" is not an existing file', - file))); + '%s path "%s" is not an existing file', ilk, file))); return; } fs.readFile(filePath, 'utf8', function (readErr, content) { @@ -222,7 +283,7 @@ function _addMetadatumFromFile(metadata, key, file, from, cb) { cb(readErr); return; } - _addMetadatum(metadata, key, content, from, cb); + _addMetadatum(ilk, metadata, key, content, from, cb); }); }); }); @@ -259,6 +320,19 @@ function do_create_instance(subcmd, opts, args, cb) { next(); }); }, + function loadTags(ctx, next) { + tagsFromOpts(opts, self.log, function (err, tags) { + if (err) { + next(err); + return; + } + if (tags) { + log.trace({tags: tags}, 'tags loaded from opts'); + ctx.tags = tags; + } + next(); + }); + }, function getImg(ctx, next) { var _opts = { name: args[0], @@ -319,6 +393,11 @@ function do_create_instance(subcmd, opts, args, cb) { createOpts['metadata.'+key] = ctx.metadata[key]; }); } + if (ctx.tags) { + Object.keys(ctx.tags).forEach(function (key) { + createOpts['tag.'+key] = ctx.tags[key]; + }); + } for (var i = 0; i < opts._order.length; i++) { var opt = opts._order[i]; @@ -436,13 +515,13 @@ do_create_instance.options = [ names: ['metadata', 'm'], type: 'arrayOfString', helpArg: 'DATA', - help: 'Add metadata to when creating the instance. Metadata are ' + - 'key/value pairs available on the instance API object on the ' + + help: 'Add metadata when creating the instance. Metadata are ' + + 'key/value pairs available on the instance API object as the ' + '"metadata" field, and inside the instance via the "mdata-*" ' + 'commands. DATA is one of: a "key=value" string (bool and ' + 'numeric "value" are converted to that type), a JSON object ' + '(if first char is "{"), or a "@FILE" to have metadata be ' + - 'loaded from FILE. This option cal be used multiple times.' + 'loaded from FILE. This option can be used multiple times.' }, { names: ['metadata-file', 'M'], @@ -458,14 +537,25 @@ do_create_instance.options = [ 'Joyent-provided images, the user-script is run at every boot ' + 'of the instance. This is a shortcut for `-M user-script=FILE`.' }, + { + names: ['tag', 't'], + type: 'arrayOfString', + helpArg: 'TAG', + help: 'Add a tag when creating the instance. Tags are ' + + 'key/value pairs available on the instance API object as the ' + + '"tags" field. TAG is one of: a "key=value" string (bool and ' + + 'numeric "value" are converted to that type), a JSON object ' + + '(if first char is "{"), or a "@FILE" to have tags be ' + + 'loaded from FILE. This option can be used multiple times.' + }, // XXX arrayOfCommaSepString dashdash type //{ // names: ['networks', 'nets'], // type: 'arrayOfCommaSepString', // help: 'One or more (comma-separated) networks IDs.' //}, - // XXX tag // XXX locality: near, far + { group: 'Other options' }, @@ -499,7 +589,7 @@ do_create_instance.help = ( ); do_create_instance.helpOpts = { - maxHelpCol: 25 + maxHelpCol: 18 }; do_create_instance.aliases = ['create']; @@ -508,3 +598,4 @@ do_create_instance.aliases = ['create']; module.exports = do_create_instance; do_create_instance.metadataFromOpts = metadataFromOpts; // export for testing +do_create_instance.tagsFromOpts = tagsFromOpts; // export for testing diff --git a/test/integration/cli-manage-workflow.test.js b/test/integration/cli-manage-workflow.test.js index cbe235b..3e07c77 100644 --- a/test/integration/cli-manage-workflow.test.js +++ b/test/integration/cli-manage-workflow.test.js @@ -147,6 +147,7 @@ test('triton manage workflow', opts, function (tt) { '-wj', '-m', 'foo=bar', '--script', __dirname + '/script-log-boot.sh', + '--tag', 'blah=bling', '-n', INST_ALIAS, imgId, pkgId ]; @@ -168,6 +169,7 @@ test('triton manage workflow', opts, function (tt) { t.equal(lines[0].id, lines[1].id, 'correct UUID given'); t.equal(lines[0].metadata.foo, 'bar', 'foo metadata set'); t.ok(lines[0].metadata['user-script'], 'user-script set'); + t.equal(lines[0].tags.blah, 'bling', 'blah tag set'); t.equal(lines[1].state, 'running', 'correct machine state'); t.end(); diff --git a/test/unit/metadataFromOpts.test.js b/test/unit/metadataFromOpts.test.js index 8dc7d18..b49c91e 100644 --- a/test/unit/metadataFromOpts.test.js +++ b/test/unit/metadataFromOpts.test.js @@ -137,8 +137,13 @@ var cases = [ argv: ['triton', 'create', '-m', '@' + __dirname + '/corpus/metadata-illegal-types.json'], expect: { - /* JSSTYLED */ - err: /invalid metadata value type: must be one of string, number, boolean: array=\[1,2,3\]/ + err: [ + /* jsl:ignore */ + /invalid metadata value type/, + /\(from .*corpus\/metadata-illegal-types.json\)/, + /must be one of string/ + /* jsl:end */ + ] } }, { diff --git a/test/unit/tagsFromOpts.test.js b/test/unit/tagsFromOpts.test.js new file mode 100644 index 0000000..9de3011 --- /dev/null +++ b/test/unit/tagsFromOpts.test.js @@ -0,0 +1,196 @@ +/* + * 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) 2015, Joyent, Inc. + */ + +/* + * Unit tests for `tagsFromOpts()` used by `triton create ...`. + */ + +var assert = require('assert-plus'); +var dashdash = require('dashdash'); +var format = require('util').format; +var test = require('tape'); + +var tagsFromOpts = require('../../lib/do_create_instance').tagsFromOpts; + + +// ---- globals + +var log = require('../lib/log'); + +var debug = function () {}; +// debug = console.warn; + + +// ---- test cases + +var OPTIONS = [ + { + names: ['tag', 't'], + type: 'arrayOfString' + } +]; + +var cases = [ + { + argv: ['triton', 'create', '-t', 'foo=bar'], + expect: { + tags: {foo: 'bar'} + } + }, + { + argv: ['triton', 'create', '--tag', 'foo=bar'], + expect: { + tags: {foo: 'bar'} + } + }, + { + argv: ['triton', 'create', '-t', 'foo=bar', '-t', 'bling=bloop'], + expect: { + tags: { + foo: 'bar', + bling: 'bloop' + } + } + }, + { + argv: ['triton', 'create', + '-t', 'num=42', + '-t', 'pi=3.14', + '-t', 'yes=true', + '-t', 'no=false', + '-t', 'array=[1,2,3]'], + expect: { + tags: { + num: 42, + pi: 3.14, + yes: true, + no: false, + array: '[1,2,3]' + } + } + }, + + { + argv: ['triton', 'create', + '-t', '@' + __dirname + '/corpus/metadata.json'], + expect: { + tags: { + 'foo': 'bar', + 'one': 'four', + 'num': 42 + } + } + }, + { + argv: ['triton', 'create', + '-t', '@' + __dirname + '/corpus/metadata.kv'], + expect: { + tags: { + 'foo': 'bar', + 'one': 'four', + 'num': 42 + } + } + }, + { + argv: ['triton', 'create', + '-t', '@' + __dirname + '/corpus/metadata-illegal-types.json'], + expect: { + err: [ + /* jsl:ignore */ + /invalid tag value type/, + /\(from .*corpus\/metadata-illegal-types.json\)/, + /must be one of string/ + /* jsl:end */ + ] + } + }, + { + argv: ['triton', 'create', + '-t', '@' + __dirname + '/corpus/metadata-invalid-json.json'], + expect: { + err: [ + /* jsl:ignore */ + /is not valid JSON/, + /corpus\/metadata-invalid-json.json/ + /* jsl:end */ + ] + } + }, + + { + argv: ['triton', 'create', + '-t', '{"foo":"bar","num":12}'], + expect: { + tags: { + 'foo': 'bar', + 'num': 12 + } + } + } +]; + + +// ---- test driver + +test('tagsFromOpts', function (tt) { + cases.forEach(function (c, num) { + var testName = format('case %d: %s', num, c.argv.join(' ')); + tt.test(testName, function (t) { + debug('--', num); + debug('c: %j', c); + var parser = new dashdash.Parser({options: OPTIONS}); + var opts = parser.parse({argv: c.argv}); + debug('opts: %j', opts); + + // Capture stderr for warnings while running. + var stderrChunks = []; + var _oldStderrWrite = process.stderr.write; + process.stderr.write = function (s) { + stderrChunks.push(s); + }; + + tagsFromOpts(opts, log, function (err, tags) { + // Restore stderr. + process.stderr.write = _oldStderrWrite; + var stderr = stderrChunks.join(''); + + if (c.expect.err) { + var errRegexps = (Array.isArray(c.expect.err) + ? c.expect.err : [c.expect.err]); + errRegexps.forEach(function (regexp) { + assert.regexp(regexp, 'case.expect.err'); + t.ok(err, 'expected an error'); + t.ok(regexp.test(err.message), format( + 'error message matches %s, actual %j', + regexp, err.message)); + }); + } else { + t.ifError(err); + } + if (c.expect.hasOwnProperty('tags')) { + t.deepEqual(tags, c.expect.tags); + } + if (c.expect.hasOwnProperty('stderr')) { + var stderrRegexps = (Array.isArray(c.expect.stderr) + ? c.expect.stderr : [c.expect.stderr]); + stderrRegexps.forEach(function (regexp) { + assert.regexp(regexp, 'case.expect.stderr'); + t.ok(regexp.test(stderr), format( + 'error message matches %s, actual %j', + regexp, stderr)); + }); + + } + t.end(); + }); + }); + }); +});