joyent/node-triton#82 locality flag/property at creation (far/near as in sdc-createmachine)

a.k.a. "Affinity", `triton create -a,--affinity RULE ...`

CHANGES.md

@@ -5,10 +5,28 @@ Known issues:
 - `triton ssh ...` disables ssh ControlMaster to avoid issue #52.
 
 
-## 4.9.1 (not yet released)
+## 4.10.0 (not yet released)
 
-(nothing yet)
+- [#82] Affinity (a.k.a. locality hints) support for instance creation, e.g.:
 
+        # Use same server as instance 'db0':
+        triton create -a instance==db0 ...
+        triton create -a db0 ...           # shortcut for same thing
+
+        # Use different server than instance 'db0':
+        triton create -a 'instance!=db0' ...
+
+        # *Attempt* to use same server as instance 'db0', but don't fail
+        # if cannot. This is called a "non-strict" or "soft" rule.
+        triton create -a instance==~db0 ...
+
+        # *Attempt* to use a different server than instance 'db0':
+        triton create -a 'instance!=~db0' ...
+
+  "Affinity" here refers to providing rules for deciding on which server
+  a new instance should by provisioned. Rules are in terms of existing
+  instances. As a shortcut, 'inst' can be used in place of 'instance'
+  above (e.g. `triton create -a 'inst!=db0' ...`).
 
 ## 4.9.0
 

etc/triton-bash-completion-types.sh

@@ -29,7 +29,7 @@ function complete_tritonprofile {
 # The next choice is to (a) use the special `TRITON_COMPLETE` handling to
 # fetch data from the server and write out a cache file, but (b) attempt to
 # find and use that cache file without calling node.js code. The win is
-# (at least in my usage) faster response time to a <TAB>. The cost is doing
+# (at least in my usage) faster response time to a <TAB>. The cost is
 # reproducing (imperfectly) in Bash the logic for determining the Triton profile
 # info to find the cache.
 #
@@ -145,6 +145,28 @@ function complete_tritonkey {
     compgen $compgen_opts -W "$candidates" -- "$word"
 }
 
+function complete_tritonaffinityrule {
+    local word="$1"
+    candidates=$(_complete_tritondata affinityrules)
+
+    # Triton affinity rules typically have '=' in them, e.g.:
+    #       triton create -a inst==db0 ...
+    # This means we run afoul of the '=' in COMP_WORDBREAKS which results in
+    #       triton create -a inst=<TAB>
+    # leading to:
+    #       triton create -a inst=inst==
+    # The answer is to strip off at the last '=' in the returned completions.
+    if [[ "$word" == *"="* ]]; then
+        local uptolastequal
+        uptolastequal="${word%=*}="
+        compgen $compgen_opts -W "$candidates" -- "$word" \
+            | cut -c$(( ${#uptolastequal} + 1 ))-
+    else
+        compgen $compgen_opts -W "$candidates" -- "$word"
+    fi
+
+}
+
 
 function complete_tritonupdateaccountfield {
     local word="$1"

lib/cli.js

@@ -442,6 +442,33 @@ CLI.prototype._emitCompletions = function _emitCompletions(type, cb) {
                     next();
                 });
                 break;
+            case 'affinityrules':
+                /*
+                 * We exclude ids, in favour of just inst names here. The only
+                 * justification for differing from other completion types
+                 * on that is that with the additional prefixes, there would
+                 * be too many.
+                 */
+                cloudapi.listMachines({}, function (err, insts) {
+                    if (err) {
+                        next(err);
+                        return;
+                    }
+                    completions = [];
+                    insts.forEach(function (inst) {
+                        if (inst.name.indexOf(' ') === -1) {
+                            // Cannot bash complete results with spaces, so
+                            // skip them here.
+                            completions.push('inst==' + inst.name);
+                            completions.push('inst!=' + inst.name);
+                            completions.push('inst==~' + inst.name);
+                            completions.push('inst!=~' + inst.name);
+                        }
+                    });
+                    arg.completions = completions.join('\n') + '\n';
+                    next();
+                });
+                break;
             case 'networks':
                 cloudapi.listNetworks({}, function (err, nets) {
                     if (err) {

lib/do_instance/do_create.js

@@ -34,6 +34,162 @@ function do_create(subcmd, opts, args, cb) {
     var cloudapi = this.top.tritonapi.cloudapi;
 
     vasync.pipeline({arg: {}, funcs: [
+        /* BEGIN JSSTYLED */
+        /*
+         * Parse --affinity options for validity to `ctx.affinities`.
+         * Later (in `resolveLocality`) we'll translate this to locality hints
+         * that CloudAPI speaks.
+         *
+         * Some examples. Inspired by
+         * <https://docs.docker.com/swarm/scheduler/filter/#how-to-write-filter-expressions>
+         *
+         *      instance==vm1
+         *      container==vm1      # alternative to 'instance'
+         *      inst==vm1           # alternative to 'instance'
+         *      inst=vm1            # '=' is shortcut for '=='
+         *      inst!=vm1           # '!='
+         *      inst==~vm1          # '~' for soft/non-strict
+         *      inst!=~vm1
+         *
+         *      inst==vm*           # globbing (not yet supported)
+         *      inst!=/vm\d/        # regex (not yet supported)
+         *
+         *      some-tag!=db        # tags (not yet supported)
+         *
+         * Limitations:
+         * - no support for tags yet
+         * - no globbing or regex yet
+         * - we resolve name -> instance id *client-side* for now (until
+         *   CloudAPI supports that)
+         * - Triton doesn't support mixed strict and non-strict, so we error
+         *   out on that. We *could* just drop the non-strict, but that is
+         *   slightly different.
+         */
+        /* END JSSTYLED */
+        function parseAffinity(ctx, next) {
+            if (!opts.affinity) {
+                next();
+                return;
+            }
+
+            var affinities = [];
+
+            // TODO: stricter rules on the value part
+            // JSSTYLED
+            var affinityRe = /((instance|inst|container)(==~|!=~|==|!=|=~|=))?(.*?)$/;
+            for (var i = 0; i < opts.affinity.length; i++) {
+                var raw = opts.affinity[i];
+                var match = affinityRe.exec(raw);
+                if (!match) {
+                    next(new errors.UsageError(format('invalid affinity: "%s"',
+                        raw)));
+                    return;
+                }
+
+                var key = match[2];
+                if ([undefined, 'inst', 'container'].indexOf(key) !== -1) {
+                    key = 'instance';
+                }
+                assert.equal(key, 'instance');
+                var op = match[3];
+                if ([undefined, '='].indexOf(op) !== -1) {
+                    op = '==';
+                }
+                var strict = true;
+                if (op[op.length - 1] === '~') {
+                    strict = false;
+                    op = op.slice(0, op.length - 1);
+                }
+                var val = match[4];
+
+                // Guard against mixed strictness (Triton can't handle those).
+                if (affinities.length > 0) {
+                    var lastAff = affinities[affinities.length - 1];
+                    if (strict !== lastAff.strict) {
+                        next(new errors.TritonError(format('mixed strict and '
+                            + 'non-strict affinities are not supported: '
+                            + '%j (%s) and %j (%s)',
+                            lastAff.raw,
+                            (lastAff.strict ? 'strict' : 'non-strict'),
+                            raw, (strict ? 'strict' : 'non-strict'))));
+                        return;
+                    }
+                }
+
+                affinities.push({
+                    raw: raw,
+                    key: key,
+                    op: op,
+                    strict: strict,
+                    val: val
+                });
+            }
+
+            if (affinities.length) {
+                log.trace({affinities: affinities}, 'affinities');
+                ctx.affinities = affinities;
+            }
+            next();
+        },
+
+        /*
+         * Determine `ctx.locality` according to what CloudAPI supports
+         * based on `ctx.affinities` parsed earlier.
+         */
+        function resolveLocality(ctx, next) {
+            if (!ctx.affinities) {
+                next();
+                return;
+            }
+
+            var strict;
+            var near = [];
+            var far = [];
+
+            vasync.forEachPipeline({
+                inputs: ctx.affinities,
+                func: function resolveAffinity(aff, nextAff) {
+                    assert.ok(['==', '!='].indexOf(aff.op) !== -1,
+                        'unexpected op: ' + aff.op);
+                    var nearFar = (aff.op == '==' ? near : far);
+
+                    strict = aff.strict;
+                    if (common.isUUID(aff.val)) {
+                        nearFar.push(aff.val);
+                        nextAff();
+                    } else {
+                        self.top.tritonapi.getInstance({
+                            id: aff.val,
+                            fields: ['id']
+                        }, function (err, inst) {
+                            if (err) {
+                                nextAff(err);
+                            } else {
+                                log.trace({val: aff.val, inst: inst.id},
+                                    'resolveAffinity');
+                                nearFar.push(inst.id);
+                                nextAff();
+                            }
+                        });
+                    }
+                }
+            }, function (err) {
+                if (err) {
+                    next(err);
+                    return;
+                }
+
+                ctx.locality = {
+                    strict: strict
+                };
+                if (near.length > 0) ctx.locality.near = near;
+                if (far.length > 0) ctx.locality.far = far;
+                log.trace({locality: ctx.locality}, 'resolveLocality');
+
+                next();
+            });
+        },
+
         function loadMetadata(ctx, next) {
             mat.metadataFromOpts(opts, log, function (err, metadata) {
                 if (err) {
@@ -116,6 +272,7 @@ function do_create(subcmd, opts, args, cb) {
                 }
             }, next);
         },
+
         function createInst(ctx, next) {
             var createOpts = {
                 name: opts.name,
@@ -124,6 +281,9 @@ function do_create(subcmd, opts, args, cb) {
                 networks: ctx.nets && ctx.nets.map(
                     function (net) { return net.id; })
             };
+            if (ctx.locality) {
+                createOpts.locality = ctx.locality;
+            }
             if (ctx.metadata) {
                 Object.keys(ctx.metadata).forEach(function (key) {
                     createOpts['metadata.'+key] = ctx.metadata[key];
@@ -293,7 +453,22 @@ do_create.options = [
         completionType: 'tritonnetwork'
     },
 
-    // XXX locality: near, far
+    {
+        names: ['affinity', 'a'],
+        type: 'arrayOfString',
+        helpArg: 'RULE',
+        help: 'Affinity rules for selecting a server for this instance. ' +
+            'Rules have one of the following forms: `instance==INST` (the ' +
+            'new instance must be on the same server as INST), ' +
+            '`instance!=INST` (new inst must *not* be on the same server as ' +
+            'INST), `instance==~INST` (*attempt* to place on the same server ' +
+            'as INST), or `instance!=~INST` (*attempt* to place on a server ' +
+            'other than INST\'s). `INST` is an existing instance name or ' +
+            'id. There are two shortcuts: `inst` may be used instead of ' +
+            '`instance` and `instance==INST` can be shortened to just ' +
+            '`INST`. Use this option more than once for multiple rules.',
+        completionType: 'tritonaffinityrule'
+    },
 
     {
         group: 'Other options'
@@ -328,7 +503,7 @@ do_create.help = (
 );
 
 do_create.helpOpts = {
-    maxHelpCol: 18
+    maxHelpCol: 16
 };
 
 do_create.completionArgtypes = ['tritonimage', 'tritonpackage', 'none'];

package.json

@@ -1,7 +1,7 @@
 {
   "name": "triton",
   "description": "Joyent Triton CLI and client (https://www.joyent.com/triton)",
-  "version": "4.9.1",
+  "version": "4.10.0",
   "author": "Joyent (joyent.com)",
   "dependencies": {
     "assert-plus": "0.2.0",

test/integration/cli-affinity.test.js

@@ -0,0 +1,132 @@
+/*
+ * 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 2016, Joyent, Inc.
+ */
+
+/*
+ * Test affinity/locality hints with `triton create -a RULE ...`.
+ *
+ * This is really only testable against a DC with multiple target CNs (i.e.
+ * COAL is out), and even then it is hard to test more than just basic cases
+ * without knowing some details about the CN provisioning pool.
+ */
+
+var format = require('util').format;
+var os = require('os');
+var test = require('tape');
+var vasync = require('vasync');
+
+var common = require('../../lib/common');
+var h = require('./helpers');
+
+
+// --- globals
+
+var ALIAS_PREFIX = format('nodetritontest-affinity-%s', os.hostname());
+
+var testOpts = {
+    skip: !h.CONFIG.allowWriteActions || h.CONFIG.skipAffinityTests
+};
+
+
+// --- Tests
+
+test('affinity (triton create -a RULE ...)', testOpts, function (tt) {
+    tt.comment('Add \'"skipAffinityTests":true\' to test/config.json if ' +
+        'this target DC does not have multiple provisionable CNs (e.g. COAL).');
+
+    // TODO: `triton rm -f` would be helpful for this
+    tt.test('  setup: rm existing insts ' + ALIAS_PREFIX + '*', function (t) {
+        // Cheat and use the current SNAFU behaviour that 'name=foo' matches
+        // all VMs *prefixed* with "foo".
+        h.safeTriton(t, ['inst', 'list', '-j', 'name='+ALIAS_PREFIX],
+                function (err, stdout) {
+            var instsToRm = h.jsonStreamParse(stdout);
+            if (instsToRm.length === 0) {
+                t.end();
+                return;
+            }
+            var rmCmd = ['inst', 'rm', '-w'].concat(
+                instsToRm.map(function (i) { return i.id; }));
+            h.safeTriton(t, rmCmd, function () {
+                t.ok(true, rmCmd.join(' '));
+                t.end();
+            });
+        });
+    });
+
+    var imgId;
+    tt.test('  setup: find test image', function (t) {
+        h.getTestImg(t, function (err, imgId_) {
+            t.ifError(err, 'getTestImg' + (err ? ': ' + err : ''));
+            imgId = imgId_;
+            t.end();
+        });
+    });
+
+    var pkgId;
+    tt.test('  setup: find test package', function (t) {
+        h.getTestPkg(t, function (err, pkgId_) {
+            t.ifError(err, 'getTestPkg' + (err ? ': ' + err : ''));
+            pkgId = pkgId_;
+            t.end();
+        });
+    });
+
+    var db0Alias = ALIAS_PREFIX + '-db0';
+    var db0;
+    tt.test('  setup: triton create -n db0', function (t) {
+        var argv = ['create', '-wj', '-n', db0Alias, imgId, pkgId];
+        h.safeTriton(t, argv, function (err, stdout) {
+            var lines = h.jsonStreamParse(stdout);
+            db0 = lines[1];
+            t.end();
+        });
+    });
+
+    // Test db1 being put on same server as db0.
+    var db1Alias = ALIAS_PREFIX + '-db1';
+    var db1;
+    tt.test('  setup: triton create -n db1 -a db0', function (t) {
+        var argv = ['create', '-wj', '-n', db1Alias, '-a', db0Alias,
+            imgId, pkgId];
+        h.safeTriton(t, argv, function (err, stdout) {
+            var lines = h.jsonStreamParse(stdout);
+            db1 = lines[1];
+            t.equal(db0.compute_node, db1.compute_node,
+                format('inst %s landed on same CN as inst %s: %s',
+                    db1Alias, db0Alias, db1.compute_node));
+            t.end();
+        });
+    });
+
+    // Test db2 being put on server *other* than db0.
+    var db2Alias = ALIAS_PREFIX + '-db2';
+    var db2;
+    tt.test('  setup: triton create -n db2 -a \'inst!=db0\'', function (t) {
+        var argv = ['create', '-wj', '-n', db2Alias, '-a', 'inst!='+db0Alias,
+            imgId, pkgId];
+        h.safeTriton(t, argv, function (err, stdout) {
+            var lines = h.jsonStreamParse(stdout);
+            db2 = lines[1];
+            t.notEqual(db0.compute_node, db2.compute_node,
+                format('inst %s landed on different CN (%s) as inst %s (%s)',
+                    db2Alias, db2.compute_node, db0Alias, db0.compute_node));
+            t.end();
+        });
+    });
+
+    // Remove instances. Add a test timeout, because '-w' on delete doesn't
+    // have a way to know if the attempt failed or if it is just taking a
+    // really long time.
+    tt.test('  cleanup: triton rm', {timeout: 10 * 60 * 1000}, function (t) {
+        h.safeTriton(t, ['rm', '-w', db0.id, db1.id, db2.id], function () {
+            t.end();
+        });
+    });
+});