joyent/node-triton#67 `triton create` should support assigning tags

Fixes #67.

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

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
  *
- *
  * <https://github.com/joyent/sdc-vmapi/blob/master/docs/index.md#vm-metadata>
  * 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,
+                _addMetadatumFromFile('metadata', metadata,
-                    o.value, next);
+                    '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
+ *
+ * <https://github.com/joyent/sdc-vmapi/blob/master/docs/index.md#vm-metadata>
+ * 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(
+        cb(new errors.UsageError(format(
-            'invalid KEY=VALUE metadata argument: ' + s));
+            '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(
+        cb(new errors.UsageError(format(
-            'invalid KEY=FILE metadata argument: ' + s));
+            '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',
+                    '%s path "%s" is not an existing file', ilk, file)));
-                    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 ' +
+        help: 'Add metadata when creating the instance. Metadata are ' +
-            'key/value pairs available on the instance API object on the ' +
+            '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

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

test/unit/metadataFromOpts.test.js

@@ -137,8 +137,13 @@ var cases = [
         argv: ['triton', 'create',
             '-m', '@' + __dirname + '/corpus/metadata-illegal-types.json'],
         expect: {
-            /* JSSTYLED */
+            err: [
-            err: /invalid metadata value type: must be one of string, number, boolean: array=\[1,2,3\]/
+                /* jsl:ignore */
+                /invalid metadata value type/,
+                /\(from .*corpus\/metadata-illegal-types.json\)/,
+                /must be one of string/
+                /* jsl:end */
+            ]
         }
     },
     {

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