From 0d271cb7e2f889132211c67710b8269c7ed5bd04 Mon Sep 17 00:00:00 2001 From: Josh Wilsdon Date: Tue, 16 May 2017 17:23:16 -0700 Subject: [PATCH 1/4] update to pass networks correctly when calling CreateVolume --- lib/cloudapi2.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index eeb617f..44f03c2 100644 --- a/lib/cloudapi2.js +++ b/lib/cloudapi2.js @@ -5,7 +5,7 @@ */ /* - * Copyright 2015 Joyent, Inc. + * Copyright 2017 Joyent, Inc. * * Client library for the SmartDataCenter Cloud API (cloudapi). * http://apidocs.joyent.com/cloudapi/ @@ -2318,17 +2318,27 @@ CloudApi.prototype.createVolume = function createVolume(options, cb) { assert.object(options, 'options'); assert.optionalString(options.name, 'options.name'); assert.optionalNumber(options.size, 'options.size'); - assert.optionalArrayOfUuid(options.networks, 'options.networks'); + assert.optionalArrayOfObject(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: options.networks, + networks: networkList, type: options.type } }, function (err, req, res, body) { From bc80ea442604938fdb4b2e7313f6e2af32a83b19 Mon Sep 17 00:00:00 2001 From: Josh Wilsdon Date: Tue, 16 May 2017 17:26:27 -0700 Subject: [PATCH 2/4] missing semicolon --- lib/cloudapi2.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index 44f03c2..6a3de5a 100644 --- a/lib/cloudapi2.js +++ b/lib/cloudapi2.js @@ -2329,7 +2329,7 @@ CloudApi.prototype.createVolume = function createVolume(options, cb) { // "public":true // }] var networkList = options.networks.map(function _mapNetworks(network) { - return network.id + return network.id; }); this._request({ From e7778a8f5d3ccd35aa4fe58863b29cf6d4af634c Mon Sep 17 00:00:00 2001 From: Josh Wilsdon Date: Wed, 17 May 2017 20:15:28 -0700 Subject: [PATCH 3/4] apply changes suggested through review, add test --- lib/cloudapi2.js | 18 ++----- lib/do_volume/do_create.js | 2 +- test/integration/cli-volumes.test.js | 77 ++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 15 deletions(-) diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index 6a3de5a..4ba70f8 100644 --- a/lib/cloudapi2.js +++ b/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) { diff --git a/lib/do_volume/do_create.js b/lib/do_volume/do_create.js index 7676670..6e461ee 100644 --- a/lib/do_volume/do_create.js +++ b/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); diff --git a/test/integration/cli-volumes.test.js b/test/integration/cli-volumes.test.js index e42ae3a..a11807f 100644 --- a/test/integration/cli-volumes.test.js +++ b/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(); + }); + }); + }); From 84ca33d6ea42688e94478f3d230fa240ab6b2c59 Mon Sep 17 00:00:00 2001 From: Josh Wilsdon Date: Thu, 18 May 2017 10:29:18 -0700 Subject: [PATCH 4/4] add name per review feedback --- test/integration/cli-volumes.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/cli-volumes.test.js b/test/integration/cli-volumes.test.js index a11807f..18fed4e 100644 --- a/test/integration/cli-volumes.test.js +++ b/test/integration/cli-volumes.test.js @@ -212,6 +212,8 @@ test('triton volume create ...', testOpts, function (tt) { h.triton([ 'volume', 'create', + '--name', + 'node-triton-test-volume-create-fabric-network', '--network', FABRIC_NETWORKS[0].id, '-w'