apply changes suggested through review, add test

lib/cloudapi2.js

@@ -2308,8 +2308,8 @@ CloudApi.prototype.listVolumes = function listVolumes(options, cb) {
  *      - name {String} Optional: the name 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
+ *      - networks {Array} Optional: an array that contains the uuids of 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)`
@@ -2318,27 +2318,17 @@ CloudApi.prototype.createVolume = function createVolume(options, cb) {
     assert.object(options, 'options');
     assert.optionalString(options.name, 'options.name');
     assert.optionalNumber(options.size, 'options.size');
-    assert.optionalArrayOfObject(options.networks, 'options.networks');
+    assert.optionalArrayOfUuid(options.networks, 'options.networks');
     assert.string(options.type, 'options.type');
     assert.func(cb, 'cb');
 
-    // options.networks is an array of objects that looks like:
-    // [{
-    //     "id":"2456155a-6459-47ba-9c9a-a77b4e781a5b",
-    //     "name":"sdc_nat",
-    //     "public":true
-    // }]
-    var networkList = options.networks.map(function _mapNetworks(network) {
-        return network.id;
-    });
-
     this._request({
         method: 'POST',
         path: format('/%s/volumes', this.account),
         data: {
             name: options.name,
             size: options.size,
-            networks: networkList,
+            networks: (options.networks ? options.networks : undefined),
             type: options.type
         }
     }, function (err, req, res, body) {

lib/do_volume/do_create.js

@@ -62,7 +62,7 @@ function do_create(subcmd, opts, args, cb) {
                     self.top.tritonapi.getNetwork(networkName,
                         function onGetNetwork(getNetErr, net) {
                             if (net) {
-                                ctx.networks.push(net);
+                                ctx.networks.push(net.id);
                             }
 
                             nextNet(getNetErr);

test/integration/cli-volumes.test.js

@@ -20,11 +20,15 @@ var vasync = require('vasync');
 var common = require('../../lib/common');
 var h = require('./helpers');
 
+var FABRIC_NETWORKS = [];
+
 var testOpts = {
     skip: !h.CONFIG.allowWriteActions
 };
 
+
 test('triton volume create ...', testOpts, function (tt) {
+    var currentVolume;
     var validVolumeName =
             h.makeResourceName('node-triton-test-volume-create-default');
 
@@ -37,6 +41,8 @@ test('triton volume create ...', testOpts, function (tt) {
     tt.test('  cleanup leftover resources', function (t) {
         h.triton(['volume', 'delete', '-y', '-w', validVolumeName].join(' '),
             function onDelVolume(delVolErr, stdout, stderr) {
+                // If there was nothing to delete, this will fail so that's the
+                // normal case. Too bad we don't have a --force option.
                 t.end();
             });
     });
@@ -54,6 +60,8 @@ test('triton volume create ...', testOpts, function (tt) {
             '--name',
             invalidVolumeName
         ].join(' '), function (volCreateErr, stdout, stderr) {
+            t.ok(volCreateErr, 'create should have failed' +
+                (volCreateErr ? '' : ', but succeeded'));
             t.equal(stderr.indexOf(expectedErrMsg), 0,
                 'stderr should include error message: ' + expectedErrMsg);
             t.end();
@@ -169,4 +177,73 @@ test('triton volume create ...', testOpts, function (tt) {
             });
     });
 
+    // Test that we can create a volume with a valid fabric network and the
+    // volume ends up on that network.
+
+    tt.test('  find fabric network', function (t) {
+        h.triton(['network', 'list', '-j'].join(' '),
+            function onGetNetworks(getNetworksErr, stdout, stderr) {
+                var resultsObj;
+
+                t.ifErr(getNetworksErr, 'should succeed getting network list');
+
+                // turn the JSON lines into a JSON object
+                resultsObj = JSON.parse('[' + stdout.trim().replace(/\n/g, ',')
+                    + ']');
+
+                t.ok(resultsObj.length > 0,
+                    'should find at least 1 network, found '
+                    + resultsObj.length);
+
+                FABRIC_NETWORKS = resultsObj.filter(function fabricFilter(net) {
+                    // keep only those networks that are marked as fabric=true
+                    return (net.fabric === true);
+                });
+
+                t.ok(FABRIC_NETWORKS.length > 0,
+                    'should find at least 1 fabric network, found '
+                    + FABRIC_NETWORKS.length);
+
+                t.end();
+            });
+    });
+
+    tt.test('  triton volume on fabric network', function (t) {
+        h.triton([
+            'volume',
+            'create',
+            '--network',
+            FABRIC_NETWORKS[0].id,
+            '-w'
+        ].join(' '), function (volCreateErr, stdout, stderr) {
+            t.ifErr(volCreateErr, 'volume creation should succeed');
+
+            currentVolume = JSON.parse(stdout);
+            t.end();
+        });
+    });
+
+    tt.test('  check volume was created', function (t) {
+        h.safeTriton(t, ['volume', 'get', currentVolume.name],
+            function onGetVolume(getVolErr, stdout) {
+                var volumeObj;
+
+                t.ifError(getVolErr, 'getting volume should succeed');
+
+                volumeObj = JSON.parse(stdout);
+                t.equal(volumeObj.networks[0], FABRIC_NETWORKS[0].id,
+                    'expect network to match fabric we passed');
+
+                t.end();
+            });
+    });
+
+    tt.test('  delete volume', function (t) {
+        h.triton(['volume', 'delete', '-y', '-w', currentVolume.name].join(' '),
+            function onDelVolume(delVolErr, stdout, stderr) {
+                t.ifError(delVolErr, 'deleting volume should succeed');
+                t.end();
+            });
+    });
+
 });