From c87ac670a01cad9ba352316493f08df096d00a52 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 21:43:24 -0800 Subject: [PATCH 01/26] add wait for dock removed --- lib/models/events.js | 47 ++++++++++++++++++++++++++++--------- lib/models/worker-server.js | 1 + lib/rabbitmq.js | 1 + lib/server.js | 3 ++- package.json | 1 + 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/lib/models/events.js b/lib/models/events.js index 0795602..57cafe2 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -3,21 +3,20 @@ require('loadenv')('mavis:env'); var url = require('url'); var keypather = require('keypather')(); - -var dockData = require('../models/dockData.js'); -var log = require('../logger').child({ module: 'events:docker' }); - var TaskFatalError = require('ponos').TaskFatalError; var TaskError = require('ponos').TaskError; -var rabbitMQ = require('../rabbitmq.js'); +var dockData = require('../models/dockData.js'); +var Consul = require('../models/consul.js'); +var Docker = require('../models/docker.js'); +var rabbitMQ = require('../rabbitmq.js'); +var log = require('../logger').child({ module: 'events:docker' }); /** * Module used to handle runnable events */ var Events = module.exports = {}; - /** * Handles docker `die` events. * updates container build counts @@ -87,14 +86,40 @@ Events.handleUnhealthy = function (data, cb) { var taskErr = new TaskError('Failed to delete host', err); return cb(taskErr); } - rabbitMQ.getPublisher().publish('on-dock-removed', data); - rabbitMQ.getPublisher().publish('cluster-instance-provision', { - githubId: data.githubId - }); - cb(); + var docker = new Docker(data.host); + docker.killSwarmContainer(function (err) { + if (err) { + var taskErr = new TaskError('Failed to kill swarm container', err); + return cb(taskErr); + } + + rabbitMQ.getPublisher().publish('cluster-instance-provision', { + githubId: data.githubId + }); + rabbitMQ.getPublisher().publish('wait-for-dock-removed', { + dockerUrl: data.host + }); + cb() + }) }); }; +/** + * waits for dock to be removed form consul + * then publish on-dock-removed event + * @param {Object} data job data + * @param {String} data.dockerUrl url to check for format: http://10.0.0.1:4242 + * @param {Function} cb (err) + */ +Events.handleWaitForDockRemoved = function (data, cb) { + log.debug({ data: data }, 'Events.handleWaitForDockRemoved'); + Consul.waitForDockRemoved(data.host, function (err) { + if (err) { return cb(err); } + + rabbitMQ.getPublisher().publish('on-dock-removed', data); + }) +} + /** * Determine type of container from given event data. * @param {Object} data Event data. diff --git a/lib/models/worker-server.js b/lib/models/worker-server.js index 06e5be4..5630978 100644 --- a/lib/models/worker-server.js +++ b/lib/models/worker-server.js @@ -27,6 +27,7 @@ WorkerServer.listen = function (cb) { var tasks = { 'on-dock-unhealthy': require('../workers/on-dock-unhealthy.js'), + 'wait-for-dock-removed': require('../workers/wait-for-dock-removed.js'), 'container.life-cycle.died': require('../workers/container.life-cycle.died.js'), 'docker.events-stream.connected': require('../workers/docker.events-stream.connected.js'), 'docker.events-stream.disconnected': require('../workers/docker.events-stream.disconnected.js'), diff --git a/lib/rabbitmq.js b/lib/rabbitmq.js index cbf76d1..1e4876a 100644 --- a/lib/rabbitmq.js +++ b/lib/rabbitmq.js @@ -52,6 +52,7 @@ RabbitMQ.prototype.create = function (cb) { this._publisher = new Hermes(put({ queues: [ 'on-dock-removed', + 'wait-for-dock-removed', 'cluster-instance-provision' ], }, opts)) diff --git a/lib/server.js b/lib/server.js index 862a8f5..28065bf 100644 --- a/lib/server.js +++ b/lib/server.js @@ -14,7 +14,7 @@ if (process.env.NEWRELIC_KEY) { require('newrelic'); } var Redis = require('./models/redis.js'); var rabbitMQ = require('./rabbitmq.js'); var WorkerServer = require('./models/worker-server.js'); - +var Docker = require('./models/docker.js'); module.exports = Server; @@ -36,6 +36,7 @@ Server.prototype.start = function (cb) { monitor.startSocketsMonitor(); docksMonitor.start(); Redis.connect(); + Docker.loadCerts(); this.server = app.listen(process.env.PORT, function(err) { if (err) { log.fatal({ err: err }, 'Error starting server. Exiting'); diff --git a/package.json b/package.json index f1dc80f..a6f77ab 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "body-parser": "^1.13.3", "boom": "^2.6.1", "bunyan": "^1.4.0", + "consul": "^0.19.0", "error-cat": "^1.4.0", "express": "^4.13.3", "express-bunyan-logger": "^1.1.1", From 73098263fc575dffc2440c596646032107d71676 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 21:46:29 -0800 Subject: [PATCH 02/26] add missing files --- lib/models/consul.js | 47 ++++++++++++++++++ lib/models/docker.js | 73 ++++++++++++++++++++++++++++ lib/workers/wait-for-dock-removed.js | 17 +++++++ 3 files changed, 137 insertions(+) create mode 100644 lib/models/consul.js create mode 100644 lib/models/docker.js create mode 100644 lib/workers/wait-for-dock-removed.js diff --git a/lib/models/consul.js b/lib/models/consul.js new file mode 100644 index 0000000..09e4000 --- /dev/null +++ b/lib/models/consul.js @@ -0,0 +1,47 @@ +/** + * Consul API requests + * @module lib/models/Consul + */ +'use strict'; +require('loadenv')('mavis:env'); + +var consul = require('consul')({ + host: process.env.CONSUL_HOST + port: process.env.CONSUL_PORT +}); +var put = require('101/put'); +var fs = require('fs'); +var join = require('path').join; +var url = require('url'); + +var log = require('./logger.js')(__filename); + +module.exports = Consul; + +/** + * class used to talk to Consul + */ +function Consul () { } + +/** + * waits for dock to be removed from consul + * will cb with TaskError if dock still exist + * @param {String} host docker host to check for + * @param {Function} cb (err) + */ +Consul.prototype.waitForDockRemoved = function (host, cb) { + var host = url.parse(host).host; + var logData = { host: host }; + log.info(logData, 'Consul.prototype.waitForDockRemoved'); + consul.kv.get('swarm/docker/swarm/nodes/' + host, function(err, result) { + if (err) { return cb(err); } + // if we have a result that means the key still exist, cb with error + if (result) { + var taskErr = new TaskError('dock still exists', err); + return cb(taskErr); + } + + log.trace(logData, 'dock as been removed'); + return cb(null); + }); +}; diff --git a/lib/models/docker.js b/lib/models/docker.js new file mode 100644 index 0000000..2d96004 --- /dev/null +++ b/lib/models/docker.js @@ -0,0 +1,73 @@ +/** + * Docker API requests + * @module lib/models/docker + */ +'use strict'; +require('loadenv')('mavis:env'); + +var Dockerode = require('dockerode'); +var put = require('101/put'); +var fs = require('fs'); +var join = require('path').join; +var url = require('url'); + +var log = require('./logger.js')(__filename); + +var certs = {}; + +module.exports = Docker; + +/** + * class used to talk to docker + * @param {string} host docker host, format: http://hostname:hostport + */ +function Docker (host) { + var parsedHost = url.parse(host); + + this.client = new Dockerode(put({ + host: parsedHost.hostname, + port: parsedHost.port + }, certs)); + + this.logData = { + host: host + }; +} + +/** + * loads certs for docker. does not throw if failed, just logs + * sync function as this should only happen once on startup + */ +Docker.loadCerts = function () { + // try/catch is a better pattern for this, since checking to see if it exists + // and then reading files can lead to race conditions (unlikely, but still) + try { + var certPath = '/etc/ssl/docker'; + certs.ca = fs.readFileSync(join(certPath, '/ca.pem')); + certs.cert = fs.readFileSync(join(certPath, '/cert.pem')); + certs.key = fs.readFileSync(join(certPath, '/key.pem')); + log.info('Docker.loadCerts docker certificates loaded'); + } catch (err) { + log.warn({err: err}, 'Docker.loadCerts cannot load certificates for docker!!'); + throw err; + } +}; + +/** + * stop swarm docker container + * @param {Function} cb (err) + */ +Docker.prototype.killSwarmContainer = function (cb) { + var self = this; + log.info(self.logData, 'Docker.prototype.killSwarmContainer'); + + self.client.getContainer('swarm').kill(function (err) { + if (err) { + log.error(put({ err: err }, self.logData), 'killSwarmContainer error killing container'); + return cb(err); + } + + log.trace(self.logData, 'killSwarmContainer killing container success'); + cb(null) + }) +}; diff --git a/lib/workers/wait-for-dock-removed.js b/lib/workers/wait-for-dock-removed.js new file mode 100644 index 0000000..0b17af6 --- /dev/null +++ b/lib/workers/wait-for-dock-removed.js @@ -0,0 +1,17 @@ +/** + * Handles `wait-for-dock-removed` event + * @module lib/workers/wait-for-dock-removed + */ +'use strict'; + +var Promise = require('bluebird'); +var Events = Promise.promisifyAll(require('../models/events.js')); +var log = require('../logger').child({ module: 'workers' }); + +module.exports = function (job) { + return Events.handleWaitForDockRemoved(job) + .catch(function (err) { + log.error({ err: err }, 'wait-for-dock-removed error'); + throw err; + }); +}; From 9338b95e42e0486ba42385672fc4b1e682b8f6e0 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 21:48:28 -0800 Subject: [PATCH 03/26] add dockerode --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index a6f77ab..9b9305c 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "boom": "^2.6.1", "bunyan": "^1.4.0", "consul": "^0.19.0", + "dockerode": "^2.2.6", "error-cat": "^1.4.0", "express": "^4.13.3", "express-bunyan-logger": "^1.1.1", From 28a49a5880f66dfb4dae69e1883d1f5daf3f8e10 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 21:51:04 -0800 Subject: [PATCH 04/26] fix logger path --- lib/models/consul.js | 2 +- lib/models/docker.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index 09e4000..30ae49c 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -14,7 +14,7 @@ var fs = require('fs'); var join = require('path').join; var url = require('url'); -var log = require('./logger.js')(__filename); +var log = require('../logger').child({ module: 'consul' }); module.exports = Consul; diff --git a/lib/models/docker.js b/lib/models/docker.js index 2d96004..f3221e9 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -11,7 +11,7 @@ var fs = require('fs'); var join = require('path').join; var url = require('url'); -var log = require('./logger.js')(__filename); +var log = require('../logger').child({ module: 'docker' }); var certs = {}; From 84a67f57f03fc3e2b96f33dc885f2e0a37893789 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 22:12:56 -0800 Subject: [PATCH 05/26] fix lint errors --- lib/models/consul.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index 30ae49c..f678b0c 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -6,13 +6,11 @@ require('loadenv')('mavis:env'); var consul = require('consul')({ - host: process.env.CONSUL_HOST + host: process.env.CONSUL_HOST, port: process.env.CONSUL_PORT }); -var put = require('101/put'); -var fs = require('fs'); -var join = require('path').join; var url = require('url'); +var TaskError = require('ponos').TaskError; var log = require('../logger').child({ module: 'consul' }); @@ -26,11 +24,11 @@ function Consul () { } /** * waits for dock to be removed from consul * will cb with TaskError if dock still exist - * @param {String} host docker host to check for + * @param {String} dockerUrl docker host to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ -Consul.prototype.waitForDockRemoved = function (host, cb) { - var host = url.parse(host).host; +Consul.prototype.waitForDockRemoved = function (dockerUrl, cb) { + var host = url.parse(dockerUrl).host; var logData = { host: host }; log.info(logData, 'Consul.prototype.waitForDockRemoved'); consul.kv.get('swarm/docker/swarm/nodes/' + host, function(err, result) { From 2ee07d77ce53b1edd3f1db14a6cf43bd7b3b4690 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 22:25:54 -0800 Subject: [PATCH 06/26] add this._subscriber --- lib/models/docker.js | 4 ++-- lib/models/events.js | 6 +++--- lib/rabbitmq.js | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/models/docker.js b/lib/models/docker.js index f3221e9..dfce6fc 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -68,6 +68,6 @@ Docker.prototype.killSwarmContainer = function (cb) { } log.trace(self.logData, 'killSwarmContainer killing container success'); - cb(null) - }) + cb(null); + }); }; diff --git a/lib/models/events.js b/lib/models/events.js index 57cafe2..d70e1b7 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -99,8 +99,8 @@ Events.handleUnhealthy = function (data, cb) { rabbitMQ.getPublisher().publish('wait-for-dock-removed', { dockerUrl: data.host }); - cb() - }) + cb(); + }); }); }; @@ -112,7 +112,7 @@ Events.handleUnhealthy = function (data, cb) { * @param {Function} cb (err) */ Events.handleWaitForDockRemoved = function (data, cb) { - log.debug({ data: data }, 'Events.handleWaitForDockRemoved'); + log.info({ data: data }, 'Events.handleWaitForDockRemoved'); Consul.waitForDockRemoved(data.host, function (err) { if (err) { return cb(err); } diff --git a/lib/rabbitmq.js b/lib/rabbitmq.js index 1e4876a..9855fc5 100644 --- a/lib/rabbitmq.js +++ b/lib/rabbitmq.js @@ -39,7 +39,8 @@ RabbitMQ.prototype.create = function (cb) { this._subscriber = new Hermes(put({ queues: [ - 'on-dock-unhealthy' + 'on-dock-unhealthy', + 'wait-for-dock-removed' ], subscribedEvents: [ 'container.life-cycle.died', From 7a429a0485adb39be91a36a8c2d87d24dd04cad3 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 22:28:17 -0800 Subject: [PATCH 07/26] make waitForDockRemoved static --- lib/models/consul.js | 2 +- lib/models/events.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index f678b0c..ae853e8 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -27,7 +27,7 @@ function Consul () { } * @param {String} dockerUrl docker host to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ -Consul.prototype.waitForDockRemoved = function (dockerUrl, cb) { +Consul.waitForDockRemoved = function (dockerUrl, cb) { var host = url.parse(dockerUrl).host; var logData = { host: host }; log.info(logData, 'Consul.prototype.waitForDockRemoved'); diff --git a/lib/models/events.js b/lib/models/events.js index d70e1b7..8bc4768 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -117,8 +117,8 @@ Events.handleWaitForDockRemoved = function (data, cb) { if (err) { return cb(err); } rabbitMQ.getPublisher().publish('on-dock-removed', data); - }) -} + }); +}; /** * Determine type of container from given event data. From d87ff95274ddefd1903ede0f9955486c7e5e6038 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 22:33:26 -0800 Subject: [PATCH 08/26] add proper key --- lib/models/events.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/events.js b/lib/models/events.js index 8bc4768..2cdcfb7 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -113,7 +113,7 @@ Events.handleUnhealthy = function (data, cb) { */ Events.handleWaitForDockRemoved = function (data, cb) { log.info({ data: data }, 'Events.handleWaitForDockRemoved'); - Consul.waitForDockRemoved(data.host, function (err) { + Consul.waitForDockRemoved(data.dockerUrl, function (err) { if (err) { return cb(err); } rabbitMQ.getPublisher().publish('on-dock-removed', data); From bba8f96214affdeafb46d47c6b477fdcdbe4971e Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 22:39:20 -0800 Subject: [PATCH 09/26] use async --- lib/workers/wait-for-dock-removed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/wait-for-dock-removed.js b/lib/workers/wait-for-dock-removed.js index 0b17af6..e8c5318 100644 --- a/lib/workers/wait-for-dock-removed.js +++ b/lib/workers/wait-for-dock-removed.js @@ -9,7 +9,7 @@ var Events = Promise.promisifyAll(require('../models/events.js')); var log = require('../logger').child({ module: 'workers' }); module.exports = function (job) { - return Events.handleWaitForDockRemoved(job) + return Events.handleWaitForDockRemovedAsync(job) .catch(function (err) { log.error({ err: err }, 'wait-for-dock-removed error'); throw err; From ff37cde6909cbd7866540472a3c676f30e2c5b3a Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 23:03:05 -0800 Subject: [PATCH 10/26] add delay --- configs/.env | 4 ++++ lib/models/consul.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/configs/.env b/configs/.env index 7724b2a..d689cbc 100644 --- a/configs/.env +++ b/configs/.env @@ -18,3 +18,7 @@ MONITOR_INTERVAL=60000 # Logging configuration LOG_LEVEL=trace + +# ponos +WORKER_MAX_RETRY_DELAY=432000000 +WORKER_MIN_RETRY_DELAY=250 \ No newline at end of file diff --git a/lib/models/consul.js b/lib/models/consul.js index ae853e8..710add2 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -35,7 +35,7 @@ Consul.waitForDockRemoved = function (dockerUrl, cb) { if (err) { return cb(err); } // if we have a result that means the key still exist, cb with error if (result) { - var taskErr = new TaskError('dock still exists', err); + var taskErr = new TaskError('wait-for-dock-removed', 'dock still exists', result); return cb(taskErr); } From bce051a02508751220e3f887501090a3ddb6cf47 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 23:14:17 -0800 Subject: [PATCH 11/26] add missing cb --- lib/models/events.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/models/events.js b/lib/models/events.js index 2cdcfb7..08b672a 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -117,6 +117,7 @@ Events.handleWaitForDockRemoved = function (data, cb) { if (err) { return cb(err); } rabbitMQ.getPublisher().publish('on-dock-removed', data); + cb(); }); }; From 6e1f80f699cb41ef83e74cc28ceca1d9a4a64832 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 23:19:31 -0800 Subject: [PATCH 12/26] make 30 second interval --- configs/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/.env b/configs/.env index d689cbc..b14589f 100644 --- a/configs/.env +++ b/configs/.env @@ -20,5 +20,5 @@ MONITOR_INTERVAL=60000 LOG_LEVEL=trace # ponos -WORKER_MAX_RETRY_DELAY=432000000 +WORKER_MAX_RETRY_DELAY=30000 WORKER_MIN_RETRY_DELAY=250 \ No newline at end of file From 3e62de65ada95f461081191b478f1e2beb0bbdf5 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Fri, 4 Dec 2015 23:24:07 -0800 Subject: [PATCH 13/26] make min delay 1/3 time --- configs/.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/.env b/configs/.env index b14589f..d614f24 100644 --- a/configs/.env +++ b/configs/.env @@ -21,4 +21,4 @@ LOG_LEVEL=trace # ponos WORKER_MAX_RETRY_DELAY=30000 -WORKER_MIN_RETRY_DELAY=250 \ No newline at end of file +WORKER_MIN_RETRY_DELAY=10000 \ No newline at end of file From a414d88dd0b33e11bba7329d0af8bbc5fc10ce0d Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 14:29:20 -0800 Subject: [PATCH 14/26] make dock-removed a published event --- lib/models/events.js | 4 ++-- lib/rabbitmq.js | 4 +++- test/functional/workers/on-dock-unhealthy.js | 7 +++++-- test/unit/workers/on-dock-unhealthy.js | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/models/events.js b/lib/models/events.js index 08b672a..d9afd56 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -106,7 +106,7 @@ Events.handleUnhealthy = function (data, cb) { /** * waits for dock to be removed form consul - * then publish on-dock-removed event + * then publish dock-removed event * @param {Object} data job data * @param {String} data.dockerUrl url to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) @@ -116,7 +116,7 @@ Events.handleWaitForDockRemoved = function (data, cb) { Consul.waitForDockRemoved(data.dockerUrl, function (err) { if (err) { return cb(err); } - rabbitMQ.getPublisher().publish('on-dock-removed', data); + rabbitMQ.getPublisher().publish('dock-removed', data); cb(); }); }; diff --git a/lib/rabbitmq.js b/lib/rabbitmq.js index 9855fc5..7fcfced 100644 --- a/lib/rabbitmq.js +++ b/lib/rabbitmq.js @@ -52,10 +52,12 @@ RabbitMQ.prototype.create = function (cb) { this._publisher = new Hermes(put({ queues: [ - 'on-dock-removed', 'wait-for-dock-removed', 'cluster-instance-provision' ], + publishedEvents: [ + 'dock-removed' + ] }, opts)) // connect publisher only since ponos is handling subscriber .connect(cb) diff --git a/test/functional/workers/on-dock-unhealthy.js b/test/functional/workers/on-dock-unhealthy.js index 2964fad..acb5399 100644 --- a/test/functional/workers/on-dock-unhealthy.js +++ b/test/functional/workers/on-dock-unhealthy.js @@ -45,10 +45,13 @@ describe('on-dock-unhealthy functional test', function () { }; ctx.rabbitClient = new Hermes(put({ queues: [ + 'wait-for-dock-removed', 'on-dock-unhealthy', - 'on-dock-removed', 'cluster-instance-provision' ], + subscribedEvents: [ + 'dock-removed' + ] }, opts)) // connect publisher only since ponos is handling subscriber .connect(done); @@ -83,7 +86,7 @@ describe('on-dock-unhealthy functional test', function () { cb(); count.next(); }); - ctx.rabbitClient.subscribe('on-dock-removed', function (data, cb) { + ctx.rabbitClient.subscribe('dock-removed', function (data, cb) { expect(data.host).to.equal(testHost); cb(); dockData.getAllDocks(function (err, data) { diff --git a/test/unit/workers/on-dock-unhealthy.js b/test/unit/workers/on-dock-unhealthy.js index 27b6e28..0dd0822 100644 --- a/test/unit/workers/on-dock-unhealthy.js +++ b/test/unit/workers/on-dock-unhealthy.js @@ -82,7 +82,7 @@ describe('on-dock-unhealthy unit test', function () { expect(dockData.deleteHost.getCall(0).args[0]).to.equal('http://10.12.12.11:4242'); dockData.deleteHost.restore(); - expect(rabbitMQ._publisher.publish.getCall(0).args[0]).to.equal('on-dock-removed'); + expect(rabbitMQ._publisher.publish.getCall(0).args[0]).to.equal('dock-removed'); expect(rabbitMQ._publisher.publish.getCall(0).args[1]).to.deep.equal({ host: 'http://10.12.12.11:4242', githubId: 12312 From fc7bea10ebafa34c9329b27986310ce2281984ee Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 17:53:53 -0800 Subject: [PATCH 15/26] WIP, not ready for review --- configs/.env | 21 ++--- configs/.env.test | 3 +- lib/models/consul.js | 17 ++-- lib/models/docker.js | 14 ++-- lib/models/events.js | 5 +- lib/workers/wait-for-dock-removed.js | 13 ++- test/fixtures/certs/ca.pem | 1 + test/fixtures/certs/cert.pem | 1 + test/fixtures/certs/key.pem | 1 + test/unit/models/consul.js | 67 +++++++++++++++ test/unit/models/docker.js | 95 ++++++++++++++++++++++ test/unit/models/events.js | 68 +++++++++++++++- test/unit/server.js | 4 + test/unit/workers/on-dock-unhealthy.js | 79 +++++++++++++----- test/unit/workers/wait-for-dock-removed.js | 65 +++++++++++++++ 15 files changed, 401 insertions(+), 53 deletions(-) create mode 100644 test/fixtures/certs/ca.pem create mode 100644 test/fixtures/certs/cert.pem create mode 100644 test/fixtures/certs/key.pem create mode 100644 test/unit/models/consul.js create mode 100644 test/unit/models/docker.js create mode 100644 test/unit/workers/wait-for-dock-removed.js diff --git a/configs/.env b/configs/.env index d614f24..560ada8 100644 --- a/configs/.env +++ b/configs/.env @@ -1,19 +1,20 @@ -PORT=4000 -REDIS_HOST_KEYS="dockerHosts:" BUILD_WEIGHT=10 CONTAINER_WEIGHT=1 +DOCKER_CERT_PATH=/etc/ssl/docker HISTORY_WEIGHT=10 +IMAGE_BUILDER=runnable/image-builder +IMAGE_BUILDER_LABEL=image-builder-container LOG=true -IMAGE_BUILDER="runnable/image-builder" -IMAGE_BUILDER_LABEL="image-builder-container" -RUNNABLE_REGISTRY="registry.runnable.com" -ROLLBAR_KEY="f4888bcd48be45cabd42627dfed87bba" -ROLLBAR_OPTIONS_BRANCH="master" -NEWRELIC_NAME='mavis' -NEWRELIC_LEVEL='error' +NEWRELIC_LEVEL=error +NEWRELIC_NAME=mavis +PORT=4000 +REDIS_HOST_KEYS=dockerHosts: +ROLLBAR_KEY=f4888bcd48be45cabd42627dfed87bba +ROLLBAR_OPTIONS_BRANCH=master +RUNNABLE_REGISTRY=registry.runnable.com # Monitor-dog configuration -MONITOR_PREFIX="mavis" +MONITOR_PREFIX=mavis MONITOR_INTERVAL=60000 # Logging configuration diff --git a/configs/.env.test b/configs/.env.test index 4fa9185..899d901 100644 --- a/configs/.env.test +++ b/configs/.env.test @@ -4,4 +4,5 @@ REDIS_IPADDRESS=127.0.0.1 LOG_LEVEL=fatal RABBITMQ_HOSTNAME=localhost RABBITMQ_PASSWORD=guest -RABBITMQ_USERNAME=guest \ No newline at end of file +RABBITMQ_USERNAME=guest +DOCKER_CERT_PATH=./test/fixtures/certs diff --git a/lib/models/consul.js b/lib/models/consul.js index 710add2..ce850f6 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -5,10 +5,6 @@ 'use strict'; require('loadenv')('mavis:env'); -var consul = require('consul')({ - host: process.env.CONSUL_HOST, - port: process.env.CONSUL_PORT -}); var url = require('url'); var TaskError = require('ponos').TaskError; @@ -19,7 +15,16 @@ module.exports = Consul; /** * class used to talk to Consul */ -function Consul () { } +function Consul () {} + +/** + * singleton consul client + * @type {Object} + */ +Consul._client = require('consul')({ + host: process.env.CONSUL_HOST, + port: process.env.CONSUL_PORT +}); /** * waits for dock to be removed from consul @@ -31,7 +36,7 @@ Consul.waitForDockRemoved = function (dockerUrl, cb) { var host = url.parse(dockerUrl).host; var logData = { host: host }; log.info(logData, 'Consul.prototype.waitForDockRemoved'); - consul.kv.get('swarm/docker/swarm/nodes/' + host, function(err, result) { + Consul._client.kv.get('swarm/docker/swarm/nodes/' + host, function(err, result) { if (err) { return cb(err); } // if we have a result that means the key still exist, cb with error if (result) { diff --git a/lib/models/docker.js b/lib/models/docker.js index dfce6fc..5a2eb20 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -19,18 +19,18 @@ module.exports = Docker; /** * class used to talk to docker - * @param {string} host docker host, format: http://hostname:hostport + * @param {string} dockerUrl format: http://hostname:hostport */ -function Docker (host) { - var parsedHost = url.parse(host); +function Docker (dockerUrl) { + var parsedHost = url.parse(dockerUrl); this.client = new Dockerode(put({ - host: parsedHost.hostname, + dockerUrl: parsedHost.hostname, port: parsedHost.port }, certs)); this.logData = { - host: host + dockerUrl: dockerUrl }; } @@ -42,13 +42,13 @@ Docker.loadCerts = function () { // try/catch is a better pattern for this, since checking to see if it exists // and then reading files can lead to race conditions (unlikely, but still) try { - var certPath = '/etc/ssl/docker'; + var certPath = process.env.DOCKER_CERT_PATH; certs.ca = fs.readFileSync(join(certPath, '/ca.pem')); certs.cert = fs.readFileSync(join(certPath, '/cert.pem')); certs.key = fs.readFileSync(join(certPath, '/key.pem')); log.info('Docker.loadCerts docker certificates loaded'); } catch (err) { - log.warn({err: err}, 'Docker.loadCerts cannot load certificates for docker!!'); + log.warn({ err: err }, 'Docker.loadCerts cannot load certificates for docker'); throw err; } }; diff --git a/lib/models/events.js b/lib/models/events.js index d9afd56..25dde50 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -2,6 +2,7 @@ require('loadenv')('mavis:env'); var url = require('url'); +var isString = require('101/is-string'); var keypather = require('keypather')(); var TaskFatalError = require('ponos').TaskFatalError; var TaskError = require('ponos').TaskError; @@ -83,13 +84,13 @@ Events.handleUnhealthy = function (data, cb) { } dockData.deleteHost(data.host, function (err) { if (err) { - var taskErr = new TaskError('Failed to delete host', err); + var taskErr = new TaskError('handleUnhealthy', 'Failed to delete host', err); return cb(taskErr); } var docker = new Docker(data.host); docker.killSwarmContainer(function (err) { if (err) { - var taskErr = new TaskError('Failed to kill swarm container', err); + var taskErr = new TaskError('handleUnhealthy', 'Failed to kill swarm container', err); return cb(taskErr); } diff --git a/lib/workers/wait-for-dock-removed.js b/lib/workers/wait-for-dock-removed.js index e8c5318..26bac49 100644 --- a/lib/workers/wait-for-dock-removed.js +++ b/lib/workers/wait-for-dock-removed.js @@ -4,12 +4,23 @@ */ 'use strict'; +var isString = require('101/is-string'); var Promise = require('bluebird'); +var TaskFatalError = require('ponos').TaskFatalError; + var Events = Promise.promisifyAll(require('../models/events.js')); var log = require('../logger').child({ module: 'workers' }); module.exports = function (job) { - return Events.handleWaitForDockRemovedAsync(job) + return Promise.resolve() + .then(function validateArguments () { + if (!isString(job.dockerUrl)) { + throw new TaskFatalError('missing dockerUrl'); + } + }) + .then(function () { + return Events.handleWaitForDockRemovedAsync(job); + }) .catch(function (err) { log.error({ err: err }, 'wait-for-dock-removed error'); throw err; diff --git a/test/fixtures/certs/ca.pem b/test/fixtures/certs/ca.pem new file mode 100644 index 0000000..2aefada --- /dev/null +++ b/test/fixtures/certs/ca.pem @@ -0,0 +1 @@ +ca \ No newline at end of file diff --git a/test/fixtures/certs/cert.pem b/test/fixtures/certs/cert.pem new file mode 100644 index 0000000..8c22b35 --- /dev/null +++ b/test/fixtures/certs/cert.pem @@ -0,0 +1 @@ +cert \ No newline at end of file diff --git a/test/fixtures/certs/key.pem b/test/fixtures/certs/key.pem new file mode 100644 index 0000000..1aadabe --- /dev/null +++ b/test/fixtures/certs/key.pem @@ -0,0 +1 @@ +key \ No newline at end of file diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js new file mode 100644 index 0000000..2881688 --- /dev/null +++ b/test/unit/models/consul.js @@ -0,0 +1,67 @@ +'use strict'; + +require('loadenv')(); + +var Lab = require('lab'); +var lab = exports.lab = Lab.script(); +var describe = lab.describe; +var it = lab.it; +var afterEach = lab.afterEach; +var beforeEach = lab.beforeEach; +var Code = require('code'); +var expect = Code.expect; + +var sinon = require('sinon'); +var TaskError = require('ponos').TaskError; + +var Consul = require('../../../lib/models/consul.js'); + +describe('consul unit test', function () { + describe('waitForDockRemoved', function () { + var dockerUrl = 'http://11.17.38.11:4242'; + + beforeEach(function (done) { + sinon.stub(Consul._client.kv, 'get'); + done(); + }); + + afterEach(function (done) { + Consul._client.kv.get.restore(); + done(); + }); + + it('should cb TaskError if get errd', function (done) { + Consul._client.kv.get.yieldsAsync(new Error('starcraft')); + + Consul.waitForDockRemoved(dockerUrl, function (err) { + expect(err).to.exist(); + done(); + }); + }); + + it('should cb TaskError if result exist', function (done) { + Consul._client.kv.get.yieldsAsync(null, {some: 'stuff'}); + + Consul.waitForDockRemoved(dockerUrl, function (err) { + expect(err).to.be.an.instanceof(TaskError); + done(); + }); + }); + + it('should cb with no err if result does not exist', function (done) { + Consul._client.kv.get.yieldsAsync(null, null); + + Consul.waitForDockRemoved(dockerUrl, function (err) { + expect(err).to.not.exist(); + sinon.assert.calledOnce(Consul._client.kv.get); + + sinon.assert.calledWith( + Consul._client.kv.get, + 'swarm/docker/swarm/nodes/11.17.38.11:4242' + ); + + done(); + }); + }); + }); // end waitForDockRemoved +}); \ No newline at end of file diff --git a/test/unit/models/docker.js b/test/unit/models/docker.js new file mode 100644 index 0000000..f67a67b --- /dev/null +++ b/test/unit/models/docker.js @@ -0,0 +1,95 @@ +'use strict'; + +require('loadenv')(); + +var Lab = require('lab'); +var lab = exports.lab = Lab.script(); +var describe = lab.describe; +var it = lab.it; +var afterEach = lab.afterEach; +var beforeEach = lab.beforeEach; +var Code = require('code'); +var expect = Code.expect; + +var Dockerode = require('dockerode'); +var sinon = require('sinon'); + +var Docker = require('../../../lib/models/docker.js'); + +describe('consul unit test', function () { + describe('constructor', function () { + it('should create Docker', function (done) { + var docker = new Docker('http://10.0.0.1:4242'); + expect(docker.client).to.be.an.instanceof(Dockerode); + done(); + }); + }); // end constructor + + describe('staticMethods', function () { + describe('loadCerts', function () { + it('should throw if missing certs', function (done) { + process.env.DOCKER_CERT_PATH = 'fake/path'; + + expect(function () { + Docker.loadCerts(); + }).to.throw(); + done(); + }); + + it('should load certs', function (done) { + process.env.DOCKER_CERT_PATH = './test/fixtures/certs'; + + expect(function () { + Docker.loadCerts(); + }).to.not.throw(); + done(); + }); + }); // end loadCerts + }); // end staticMethods + + describe('prototype methods', function () { + var docker; + var dockerUrl = 'http://10.0.0.1:4242'; + + beforeEach(function (done) { + docker = new Docker(dockerUrl); + done(); + }); + + describe('killSwarmContainer', function () { + beforeEach(function (done) { + docker.client = { + getContainer: sinon.stub().returnsThis(), + kill: sinon.stub() + }; + done(); + }); + + it('should cb error if kill failed', function (done) { + docker.client.kill.yieldsAsync(new Error('iceberg')); + + docker.killSwarmContainer(function (err) { + expect(err).to.exist(); + done(); + }); + }); + + it('should cb on success', function (done) { + docker.client.kill.yieldsAsync(); + + docker.killSwarmContainer(function (err) { + expect(err).to.not.exist(); + + sinon.assert.calledOnce(docker.client.getContainer); + sinon.assert.calledWith( + docker.client.getContainer, + 'swarm' + ); + sinon.assert.calledOnce(docker.client.kill); + + done(); + }); + }); + }); // end killSwarmContainer + }); // end prototype methods +}); \ No newline at end of file diff --git a/test/unit/models/events.js b/test/unit/models/events.js index 9960398..ca07f4c 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -2,16 +2,23 @@ require('loadenv')('mavis:env'); -var async = require('async'); - var Lab = require('lab'); var lab = exports.lab = Lab.script(); +var Code = require('code'); +var expect = Code.expect; +var afterEach = lab.afterEach; +var beforeEach = lab.beforeEach; + var redis = require('../../../lib/models/redis.js'); var events = require('../../../lib/models/events.js'); var dockData = require('../../../lib/models/dockData.js'); +var RabbitMQ = require('../../../lib/rabbitmq.js'); +var Consul = require('../../../lib/models/consul.js'); + +var async = require('async'); +var sinon = require('sinon'); + var host = 'http://0.0.0.0:4242'; -var Code = require('code'); -var expect = Code.expect; function dataExpect1(data, numContainers, numBuilds, host) { @@ -558,4 +565,57 @@ lab.experiment('events.js unit test', function () { }); }); // handleDockDown }); // deamon + + lab.experiment('handleWaitForDockRemoved', function () { + var publishStub; + beforeEach(function (done) { + sinon.stub(Consul, 'waitForDockRemoved'); + publishStub = sinon.stub(); + sinon.stub(RabbitMQ, 'getPublisher').returns({ + publish: publishStub + }); + done(); + }); + + afterEach(function (done) { + Consul.waitForDockRemoved.restore(); + RabbitMQ.getPublisher.restore(); + done(); + }); + + lab.test('should cb err if waitForDockRemoved failed', function (done) { + Consul.waitForDockRemoved.yieldsAsync(new Error('blue')); + + events.handleWaitForDockRemoved({}, function (err) { + expect(err).to.exist(); + done(); + }); + }); + + lab.test('should publish dock-removed', function (done) { + var dockerUrl = 'http://10.0.102.2:4242'; + Consul.waitForDockRemoved.yieldsAsync(null); + + events.handleWaitForDockRemoved({ + dockerUrl: dockerUrl + }, function (err) { + expect(err).to.not.exist(); + + sinon.assert.calledOnce(Consul.waitForDockRemoved); + sinon.assert.calledWith( + Consul.waitForDockRemoved, + dockerUrl + ); + + sinon.assert.calledOnce(publishStub); + sinon.assert.calledWith( + publishStub, + 'dock-removed', + sinon.match({ dockerUrl: dockerUrl }) + ); + + done(); + }); + }); + }); // end handleWaitForDockRemoved }); // docker events diff --git a/test/unit/server.js b/test/unit/server.js index 6062049..e5ff2ca 100644 --- a/test/unit/server.js +++ b/test/unit/server.js @@ -18,6 +18,7 @@ var docksMonitor = require('../../lib/models/docks-monitor'); var RabbitMQ = require('../../lib/rabbitmq.js'); var Server = require('../../lib/server.js'); var app = require('../../lib/app'); +var Docker = require('../../lib/models/docker.js'); describe('server.js unit test', function () { @@ -29,6 +30,7 @@ describe('server.js unit test', function () { sinon.stub(app, 'listen'); sinon.stub(RabbitMQ, 'create'); sinon.stub(WorkerServer, 'listen'); + sinon.stub(Docker, 'loadCerts'); done(); }); @@ -39,6 +41,7 @@ describe('server.js unit test', function () { app.listen.restore(); RabbitMQ.create.restore(); WorkerServer.listen.restore(); + Docker.loadCerts.restore(); done(); }); @@ -46,6 +49,7 @@ describe('server.js unit test', function () { monitor.startSocketsMonitor.returns(); docksMonitor.start.returns(); Redis.connect.returns(); + Docker.loadCerts.returns(); app.listen.yieldsAsync(); RabbitMQ.create.yieldsAsync(); WorkerServer.listen.yieldsAsync(); diff --git a/test/unit/workers/on-dock-unhealthy.js b/test/unit/workers/on-dock-unhealthy.js index 0dd0822..5f1f4bb 100644 --- a/test/unit/workers/on-dock-unhealthy.js +++ b/test/unit/workers/on-dock-unhealthy.js @@ -20,10 +20,10 @@ var Worker = require('../../../lib/workers/on-dock-unhealthy.js'); var dockData = require('../../../lib/models/dockData.js'); var rabbitMQ = require('../../../lib/rabbitmq.js'); var Events = require('../../../lib/models/events.js'); +var Docker = require('../../../lib/models/docker.js'); var onDockUnhealthy = require('../../../lib/workers/on-dock-unhealthy.js'); describe('on-dock-unhealthy unit test', function () { - it('should throw error if invalid host', function (done) { sinon.spy(Events, '_hasValidHost'); onDockUnhealthy({}) @@ -58,44 +58,79 @@ describe('on-dock-unhealthy unit test', function () { }); }); + it('should throw if killingSwarmContainer failed', function (done) { + var dockerUrl = 'http://10.12.12.11:4242'; + + sinon.spy(Events, '_hasValidHost'); + sinon.stub(dockData, 'deleteHost').yieldsAsync(null); + sinon.stub(Docker.prototype, 'killSwarmContainer').yieldsAsync(new Error('Docker Error')); + + onDockUnhealthy({ + host: dockerUrl, + }) + .then(function () { + throw new Error('Should not happen'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(TaskError); + expect(Events._hasValidHost.calledOnce).to.be.true(); + expect(dockData.deleteHost.called).to.be.true(); + expect(dockData.deleteHost.getCall(0).args[0]).to.equal(dockerUrl); + dockData.deleteHost.restore(); + Events._hasValidHost.restore(); + Docker.prototype.killSwarmContainer.restore(); + done(); + }); + }); + describe('success', function () { + beforeEach(function (done) { + sinon.spy(Events, '_hasValidHost'); + sinon.stub(dockData, 'deleteHost').yieldsAsync(null); + sinon.stub(Docker.prototype, 'killSwarmContainer').yieldsAsync(null); + done(); + }); + afterEach(function (done) { + Events._hasValidHost.restore(); + dockData.deleteHost.restore(); + Docker.prototype.killSwarmContainer.restore(); rabbitMQ._publisher = null; done(); }); - it('should delete host if valid', function (done) { - sinon.spy(Events, '_hasValidHost'); - sinon.stub(dockData, 'deleteHost').yieldsAsync(null); + + it('should emit provision and wait events', function (done) { + var dockerUrl = 'http://10.12.12.11:4242'; + rabbitMQ._publisher = { publish: function (name, data) {} }; sinon.spy(rabbitMQ._publisher, 'publish'); onDockUnhealthy({ - host: 'http://10.12.12.11:4242', + host: dockerUrl, githubId: 12312 }) .then(function () { expect(Events._hasValidHost.calledOnce).to.be.true(); - Events._hasValidHost.restore(); expect(dockData.deleteHost.called).to.be.true(); - expect(dockData.deleteHost.getCall(0).args[0]).to.equal('http://10.12.12.11:4242'); - dockData.deleteHost.restore(); - - expect(rabbitMQ._publisher.publish.getCall(0).args[0]).to.equal('dock-removed'); - expect(rabbitMQ._publisher.publish.getCall(0).args[1]).to.deep.equal({ - host: 'http://10.12.12.11:4242', - githubId: 12312 - }); - expect(rabbitMQ._publisher.publish.getCall(1).args[0]).to.equal('cluster-instance-provision'); - expect(rabbitMQ._publisher.publish.getCall(1).args[1]).to.deep.equal({ - githubId: 12312 - }); - rabbitMQ._publisher.publish.restore(); + expect(dockData.deleteHost.getCall(0).args[0]).to.equal(dockerUrl); + + sinon.assert.calledTwice(rabbitMQ._publisher.publish); + + sinon.assert.calledWith( + rabbitMQ._publisher.publish.getCall(0), + 'cluster-instance-provision', + sinon.match({ githubId: 12312 }) + ); + + sinon.assert.calledWith( + rabbitMQ._publisher.publish.getCall(1), + 'wait-for-dock-removed', + sinon.match({ dockerUrl: dockerUrl }) + ); + done(); - }) - .catch(function (err) { - throw new Error('Should not happen'); }); }); }); diff --git a/test/unit/workers/wait-for-dock-removed.js b/test/unit/workers/wait-for-dock-removed.js new file mode 100644 index 0000000..eac342f --- /dev/null +++ b/test/unit/workers/wait-for-dock-removed.js @@ -0,0 +1,65 @@ +'use strict'; +require('loadenv')(); + +var Lab = require('lab'); +var lab = exports.lab = Lab.script(); +var describe = lab.describe; +var it = lab.it; +var afterEach = lab.afterEach; +var beforeEach = lab.beforeEach; +var Code = require('code'); +var expect = Code.expect; + +var sinon = require('sinon'); +var TaskFatalError = require('ponos').TaskFatalError; + +var Events = require('../../../lib/models/events.js'); +var waitForDockRemovedWorker = require('../../../lib/workers/wait-for-dock-removed.js'); + +describe('wait-for-dock-removed unit test', function () { + describe('run', function () { + beforeEach(function (done) { + sinon.stub(Events, 'handleWaitForDockRemovedAsync'); + done(); + }); + + afterEach(function (done) { + Events.handleWaitForDockRemovedAsync.restore(); + done(); + }); + + it('should throw error if handleWaitForDockRemovedAsync failed', function (done) { + Events.handleWaitForDockRemovedAsync.throws(new Error('test')); + waitForDockRemovedWorker({ + dockerUrl: '10.0.0.1:4224', + }) + .then(function () { + throw new Error('should have thrown'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(Error); + done(); + }); + }); + + it('should throw missing dockerUrl', function (done) { + waitForDockRemovedWorker({}) + .then(function () { + throw new Error('should have thrown'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(TaskFatalError); + done(); + }); + }); + + it('should be fine if no errors', function (done) { + Events.handleWaitForDockRemovedAsync.returns(); + waitForDockRemovedWorker({ + dockerUrl: '10.0.0.1:4224' + }) + .then(done) + .catch(done); + }); + }); // end run +}); // end docker.events-stream.connected unit test From a75ab1f26edf567698b24c6558f8aef3b234b94f Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 21:06:09 -0800 Subject: [PATCH 16/26] fix unhealthy test --- configs/.env.test | 10 +- lib/models/docker.js | 3 +- lib/models/events.js | 6 +- package.json | 1 + test/functional/workers/on-dock-unhealthy.js | 124 ++++++++++++------- test/unit/models/docker.js | 3 +- test/unit/models/events.js | 2 +- 7 files changed, 94 insertions(+), 55 deletions(-) diff --git a/configs/.env.test b/configs/.env.test index 899d901..36fa68d 100644 --- a/configs/.env.test +++ b/configs/.env.test @@ -1,8 +1,10 @@ -PORT=65213 -REDIS_PORT=6379 -REDIS_IPADDRESS=127.0.0.1 +CONSUL_HOST=consul.com +CONSUL_PORT=8500 +DOCKER_CERT_PATH=./test/fixtures/certs LOG_LEVEL=fatal +PORT=65213 RABBITMQ_HOSTNAME=localhost RABBITMQ_PASSWORD=guest RABBITMQ_USERNAME=guest -DOCKER_CERT_PATH=./test/fixtures/certs +REDIS_IPADDRESS=127.0.0.1 +REDIS_PORT=6379 diff --git a/lib/models/docker.js b/lib/models/docker.js index 5a2eb20..b3edc8e 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -25,13 +25,14 @@ function Docker (dockerUrl) { var parsedHost = url.parse(dockerUrl); this.client = new Dockerode(put({ - dockerUrl: parsedHost.hostname, + host: parsedHost.hostname, port: parsedHost.port }, certs)); this.logData = { dockerUrl: dockerUrl }; + log.trace(this.logData, 'Docker constructor'); } /** diff --git a/lib/models/events.js b/lib/models/events.js index 25dde50..17295b2 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -2,7 +2,6 @@ require('loadenv')('mavis:env'); var url = require('url'); -var isString = require('101/is-string'); var keypather = require('keypather')(); var TaskFatalError = require('ponos').TaskFatalError; var TaskError = require('ponos').TaskError; @@ -117,7 +116,10 @@ Events.handleWaitForDockRemoved = function (data, cb) { Consul.waitForDockRemoved(data.dockerUrl, function (err) { if (err) { return cb(err); } - rabbitMQ.getPublisher().publish('dock-removed', data); + log.trace({ data: data }, 'handleWaitForDockRemoved publishing dock-removed'); + rabbitMQ.getPublisher().publish('dock-removed', { + host: data.dockerUrl + }); cb(); }); }; diff --git a/package.json b/package.json index 9b9305c..9272345 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "loadenv": "^1.1.0", "monitor-dog": "^1.4.1", "newrelic": "^1.22.0", + "nock": "^3.3.2", "node-dogstatsd": "0.0.6", "ponos": "^1.1.1", "redis": "^0.12.1", diff --git a/test/functional/workers/on-dock-unhealthy.js b/test/functional/workers/on-dock-unhealthy.js index acb5399..868f12c 100644 --- a/test/functional/workers/on-dock-unhealthy.js +++ b/test/functional/workers/on-dock-unhealthy.js @@ -2,91 +2,125 @@ require('loadenv')(); -var put = require('101/put'); var Lab = require('lab'); var lab = exports.lab = Lab.script(); var describe = lab.describe; var it = lab.it; -var after = lab.after; -var before = lab.before; var beforeEach = lab.beforeEach; +var afterEach = lab.afterEach; var Code = require('code'); var expect = Code.expect; var createCount = require('callback-count'); +var nock = require('nock'); +var redis = require('redis'); var Hermes = require('runnable-hermes'); var dockData = require('../../../lib/models/dockData.js'); var Server = require('../../../lib/server.js'); -var rabbitMQ = require('../../../lib/rabbitmq.js'); -var WorkerServer = require('../../../lib/models/worker-server.js'); -var redis = require('../../../lib/models/redis.js'); + +var server = new Server(); + +var publishedEvents = [ + 'container.life-cycle.died', + 'docker.events-stream.connected', + 'docker.events-stream.disconnected' +]; + +var subscribedEvents = [ + 'dock-removed' +]; + +var queues = [ + 'weave.start', + 'on-dock-unhealthy', + 'cluster-instance-provision' +]; + +var testPublisher = new Hermes({ + hostname: process.env.RABBITMQ_HOSTNAME, + password: process.env.RABBITMQ_PASSWORD, + port: process.env.RABBITMQ_PORT, + username: process.env.RABBITMQ_USERNAME, + publishedEvents: publishedEvents, + queues: queues, + name: 'testPublisher' +}); + +var testSubscriber = new Hermes({ + hostname: process.env.RABBITMQ_HOSTNAME, + password: process.env.RABBITMQ_PASSWORD, + port: process.env.RABBITMQ_PORT, + username: process.env.RABBITMQ_USERNAME, + subscribedEvents: subscribedEvents, + queues: queues, + name: 'testSubscriber' +}); + +var testRedis = redis.createClient( + process.env.REDIS_PORT, + process.env.REDIS_IPADDRESS); + describe('on-dock-unhealthy functional test', function () { var testHost = 'http://10.20.1.26:4242'; var testGihubId = 2194285; - var ctx = {}; - before(function (done) { - redis.connect(); - rabbitMQ.create(done); - }); - before(function (done) { - WorkerServer.listen(done); + beforeEach(function (done) { + // connect publisher so exchanges are generated before sever init + testPublisher.connect(done); }); - before(function (done) { - var opts = { - hostname: process.env.RABBITMQ_HOSTNAME, - password: process.env.RABBITMQ_PASSWORD, - port: process.env.RABBITMQ_PORT, - username: process.env.RABBITMQ_USERNAME, - name: 'mavis' - }; - ctx.rabbitClient = new Hermes(put({ - queues: [ - 'wait-for-dock-removed', - 'on-dock-unhealthy', - 'cluster-instance-provision' - ], - subscribedEvents: [ - 'dock-removed' - ] - }, opts)) - // connect publisher only since ponos is handling subscriber - .connect(done); + beforeEach(function (done) { + server.start(done); }); - after(function (done) { - ctx.rabbitClient.close(done); + beforeEach(function (done) { + // connect subscriber after server has started + testSubscriber.connect(done); }); lab.beforeEach(function (done) { - redis.client.flushall(done); + // nock docker call + nock(testHost) + .post('/containers/swarm/kill') + .reply(200); + + // nock consul call + nock('http://consul.com:8500') + .get('/v1/kv/swarm%2Fdocker%2Fswarm%2Fnodes%2F10.20.1.26%3A4242') + .reply(404); + + testRedis.flushall(done); }); beforeEach(function(done) { dockData.addHost(testHost, 'test,tags', done); }); - after(function (done) { - WorkerServer.stop(done); - }) - after(function (done) { - redis.disconnect(); - rabbitMQ.close(done); + afterEach(function (done) { + testSubscriber.close(done); + }); + + afterEach(function (done) { + testPublisher.close(done); + }); + + afterEach(function (done) { + server.stop(done); }); describe('on-docker-unhealthy event', function () { it('should remove host and publish two events', function (done) { var count = createCount(2, done); - ctx.rabbitClient.subscribe('cluster-instance-provision', function (data, cb) { + testSubscriber.subscribe('cluster-instance-provision', function (data, cb) { expect(data.githubId).to.equal(testGihubId); cb(); count.next(); }); - ctx.rabbitClient.subscribe('dock-removed', function (data, cb) { + + testSubscriber.subscribe('dock-removed', function (data, cb) { expect(data.host).to.equal(testHost); cb(); dockData.getAllDocks(function (err, data) { @@ -95,7 +129,7 @@ describe('on-dock-unhealthy functional test', function () { count.next(); }); }); - ctx.rabbitClient.publish('on-dock-unhealthy', { + testPublisher.publish('on-dock-unhealthy', { host: testHost, githubId: testGihubId }); diff --git a/test/unit/models/docker.js b/test/unit/models/docker.js index f67a67b..ac97e4d 100644 --- a/test/unit/models/docker.js +++ b/test/unit/models/docker.js @@ -6,7 +6,6 @@ var Lab = require('lab'); var lab = exports.lab = Lab.script(); var describe = lab.describe; var it = lab.it; -var afterEach = lab.afterEach; var beforeEach = lab.beforeEach; var Code = require('code'); var expect = Code.expect; @@ -16,7 +15,7 @@ var sinon = require('sinon'); var Docker = require('../../../lib/models/docker.js'); -describe('consul unit test', function () { +describe('docker unit test', function () { describe('constructor', function () { it('should create Docker', function (done) { var docker = new Docker('http://10.0.0.1:4242'); diff --git a/test/unit/models/events.js b/test/unit/models/events.js index ca07f4c..44bd4c0 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -611,7 +611,7 @@ lab.experiment('events.js unit test', function () { sinon.assert.calledWith( publishStub, 'dock-removed', - sinon.match({ dockerUrl: dockerUrl }) + sinon.match({ host: dockerUrl }) ); done(); From 4b4242f277cc815ad12e7bf317bb188d54afc557 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 21:12:25 -0800 Subject: [PATCH 17/26] wait-for-dock-removed.js -> dock.wait-for-removal.js --- lib/models/consul.js | 2 +- lib/models/events.js | 2 +- lib/models/worker-server.js | 2 +- lib/rabbitmq.js | 4 ++-- .../{wait-for-dock-removed.js => dock.wait-for-removal.js} | 6 +++--- .../{wait-for-dock-removed.js => dock.wait-for-removal.js} | 4 ++-- test/unit/workers/on-dock-unhealthy.js | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) rename lib/workers/{wait-for-dock-removed.js => dock.wait-for-removal.js} (80%) rename test/unit/workers/{wait-for-dock-removed.js => dock.wait-for-removal.js} (92%) diff --git a/lib/models/consul.js b/lib/models/consul.js index ce850f6..dde47d6 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -40,7 +40,7 @@ Consul.waitForDockRemoved = function (dockerUrl, cb) { if (err) { return cb(err); } // if we have a result that means the key still exist, cb with error if (result) { - var taskErr = new TaskError('wait-for-dock-removed', 'dock still exists', result); + var taskErr = new TaskError('dock.wait-for-removal', 'dock still exists', result); return cb(taskErr); } diff --git a/lib/models/events.js b/lib/models/events.js index 17295b2..dedd9a3 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -96,7 +96,7 @@ Events.handleUnhealthy = function (data, cb) { rabbitMQ.getPublisher().publish('cluster-instance-provision', { githubId: data.githubId }); - rabbitMQ.getPublisher().publish('wait-for-dock-removed', { + rabbitMQ.getPublisher().publish('dock.wait-for-removal', { dockerUrl: data.host }); cb(); diff --git a/lib/models/worker-server.js b/lib/models/worker-server.js index 5630978..72c43eb 100644 --- a/lib/models/worker-server.js +++ b/lib/models/worker-server.js @@ -27,7 +27,7 @@ WorkerServer.listen = function (cb) { var tasks = { 'on-dock-unhealthy': require('../workers/on-dock-unhealthy.js'), - 'wait-for-dock-removed': require('../workers/wait-for-dock-removed.js'), + 'dock.wait-for-removal': require('../workers/dock.wait-for-removal.js'), 'container.life-cycle.died': require('../workers/container.life-cycle.died.js'), 'docker.events-stream.connected': require('../workers/docker.events-stream.connected.js'), 'docker.events-stream.disconnected': require('../workers/docker.events-stream.disconnected.js'), diff --git a/lib/rabbitmq.js b/lib/rabbitmq.js index 7fcfced..0dd4587 100644 --- a/lib/rabbitmq.js +++ b/lib/rabbitmq.js @@ -40,7 +40,7 @@ RabbitMQ.prototype.create = function (cb) { this._subscriber = new Hermes(put({ queues: [ 'on-dock-unhealthy', - 'wait-for-dock-removed' + 'dock.wait-for-removal' ], subscribedEvents: [ 'container.life-cycle.died', @@ -52,7 +52,7 @@ RabbitMQ.prototype.create = function (cb) { this._publisher = new Hermes(put({ queues: [ - 'wait-for-dock-removed', + 'dock.wait-for-removal', 'cluster-instance-provision' ], publishedEvents: [ diff --git a/lib/workers/wait-for-dock-removed.js b/lib/workers/dock.wait-for-removal.js similarity index 80% rename from lib/workers/wait-for-dock-removed.js rename to lib/workers/dock.wait-for-removal.js index 26bac49..9c41f25 100644 --- a/lib/workers/wait-for-dock-removed.js +++ b/lib/workers/dock.wait-for-removal.js @@ -1,6 +1,6 @@ /** - * Handles `wait-for-dock-removed` event - * @module lib/workers/wait-for-dock-removed + * Handles `dock.wait-for-removal` event + * @module lib/workers/dock.wait-for-removal */ 'use strict'; @@ -22,7 +22,7 @@ module.exports = function (job) { return Events.handleWaitForDockRemovedAsync(job); }) .catch(function (err) { - log.error({ err: err }, 'wait-for-dock-removed error'); + log.error({ err: err }, 'dock.wait-for-removal error'); throw err; }); }; diff --git a/test/unit/workers/wait-for-dock-removed.js b/test/unit/workers/dock.wait-for-removal.js similarity index 92% rename from test/unit/workers/wait-for-dock-removed.js rename to test/unit/workers/dock.wait-for-removal.js index eac342f..5b4c0a9 100644 --- a/test/unit/workers/wait-for-dock-removed.js +++ b/test/unit/workers/dock.wait-for-removal.js @@ -14,9 +14,9 @@ var sinon = require('sinon'); var TaskFatalError = require('ponos').TaskFatalError; var Events = require('../../../lib/models/events.js'); -var waitForDockRemovedWorker = require('../../../lib/workers/wait-for-dock-removed.js'); +var waitForDockRemovedWorker = require('../../../lib/workers/dock.wait-for-removal.js'); -describe('wait-for-dock-removed unit test', function () { +describe('dock.wait-for-removal unit test', function () { describe('run', function () { beforeEach(function (done) { sinon.stub(Events, 'handleWaitForDockRemovedAsync'); diff --git a/test/unit/workers/on-dock-unhealthy.js b/test/unit/workers/on-dock-unhealthy.js index 5f1f4bb..e0b4d9c 100644 --- a/test/unit/workers/on-dock-unhealthy.js +++ b/test/unit/workers/on-dock-unhealthy.js @@ -126,7 +126,7 @@ describe('on-dock-unhealthy unit test', function () { sinon.assert.calledWith( rabbitMQ._publisher.publish.getCall(1), - 'wait-for-dock-removed', + 'dock.wait-for-removal', sinon.match({ dockerUrl: dockerUrl }) ); From 2dc464eead12aacc85961377a3938ccf213536b8 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 21:13:40 -0800 Subject: [PATCH 18/26] dock-removed -> dock.removed --- lib/models/events.js | 6 +++--- lib/rabbitmq.js | 2 +- test/functional/workers/on-dock-unhealthy.js | 4 ++-- test/unit/models/events.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/models/events.js b/lib/models/events.js index dedd9a3..41394aa 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -106,7 +106,7 @@ Events.handleUnhealthy = function (data, cb) { /** * waits for dock to be removed form consul - * then publish dock-removed event + * then publish dock.removed event * @param {Object} data job data * @param {String} data.dockerUrl url to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) @@ -116,8 +116,8 @@ Events.handleWaitForDockRemoved = function (data, cb) { Consul.waitForDockRemoved(data.dockerUrl, function (err) { if (err) { return cb(err); } - log.trace({ data: data }, 'handleWaitForDockRemoved publishing dock-removed'); - rabbitMQ.getPublisher().publish('dock-removed', { + log.trace({ data: data }, 'handleWaitForDockRemoved publishing dock.removed'); + rabbitMQ.getPublisher().publish('dock.removed', { host: data.dockerUrl }); cb(); diff --git a/lib/rabbitmq.js b/lib/rabbitmq.js index 0dd4587..3b2f768 100644 --- a/lib/rabbitmq.js +++ b/lib/rabbitmq.js @@ -56,7 +56,7 @@ RabbitMQ.prototype.create = function (cb) { 'cluster-instance-provision' ], publishedEvents: [ - 'dock-removed' + 'dock.removed' ] }, opts)) // connect publisher only since ponos is handling subscriber diff --git a/test/functional/workers/on-dock-unhealthy.js b/test/functional/workers/on-dock-unhealthy.js index 868f12c..d0e5d46 100644 --- a/test/functional/workers/on-dock-unhealthy.js +++ b/test/functional/workers/on-dock-unhealthy.js @@ -28,7 +28,7 @@ var publishedEvents = [ ]; var subscribedEvents = [ - 'dock-removed' + 'dock.removed' ]; var queues = [ @@ -120,7 +120,7 @@ describe('on-dock-unhealthy functional test', function () { count.next(); }); - testSubscriber.subscribe('dock-removed', function (data, cb) { + testSubscriber.subscribe('dock.removed', function (data, cb) { expect(data.host).to.equal(testHost); cb(); dockData.getAllDocks(function (err, data) { diff --git a/test/unit/models/events.js b/test/unit/models/events.js index 44bd4c0..d144d39 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -592,7 +592,7 @@ lab.experiment('events.js unit test', function () { }); }); - lab.test('should publish dock-removed', function (done) { + lab.test('should publish dock.removed', function (done) { var dockerUrl = 'http://10.0.102.2:4242'; Consul.waitForDockRemoved.yieldsAsync(null); @@ -610,7 +610,7 @@ lab.experiment('events.js unit test', function () { sinon.assert.calledOnce(publishStub); sinon.assert.calledWith( publishStub, - 'dock-removed', + 'dock.removed', sinon.match({ host: dockerUrl }) ); From c923d184c561039ac23974e1b5b2afff7ea55f4f Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sat, 5 Dec 2015 21:40:09 -0800 Subject: [PATCH 19/26] move to node 4.2.2 --- circle.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 circle.yml diff --git a/circle.yml b/circle.yml new file mode 100644 index 0000000..bad2338 --- /dev/null +++ b/circle.yml @@ -0,0 +1,6 @@ +dependencies: + override: + - nvm install 4.2.2 + - nvm alias default 4.2.2 + - npm install -g npm@2.14.7 + - npm install From 0b403257e4851c0c1f813bd88a406302420a0547 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sun, 6 Dec 2015 17:21:21 -0800 Subject: [PATCH 20/26] fix some nits, rename and error checking --- lib/models/consul.js | 2 +- lib/models/docker.js | 14 +++++++------- test/functional/workers/on-dock-unhealthy.js | 2 +- test/unit/models/consul.js | 10 +++++++--- test/unit/models/docker.js | 19 ++++++++++--------- test/unit/models/events.js | 7 ++++--- test/unit/models/redis.js | 2 +- test/unit/models/worker-server.js | 2 +- .../unit/workers/container.life-cycle.died.js | 15 ++------------- test/unit/workers/dock.wait-for-removal.js | 7 ++++--- .../workers/docker.events-stream.connected.js | 13 ++----------- .../docker.events-stream.disconnected.js | 12 ++---------- test/unit/workers/on-dock-unhealthy.js | 3 +-- 13 files changed, 43 insertions(+), 65 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index dde47d6..da9f856 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -44,7 +44,7 @@ Consul.waitForDockRemoved = function (dockerUrl, cb) { return cb(taskErr); } - log.trace(logData, 'dock as been removed'); + log.trace(logData, 'waitForDockRemoved dock as been removed'); return cb(null); }); }; diff --git a/lib/models/docker.js b/lib/models/docker.js index b3edc8e..5b2c772 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -24,15 +24,15 @@ module.exports = Docker; function Docker (dockerUrl) { var parsedHost = url.parse(dockerUrl); - this.client = new Dockerode(put({ + this._client = new Dockerode(put({ host: parsedHost.hostname, port: parsedHost.port }, certs)); - this.logData = { + this._logData = { dockerUrl: dockerUrl }; - log.trace(this.logData, 'Docker constructor'); + log.trace(this._logData, 'Docker constructor'); } /** @@ -60,15 +60,15 @@ Docker.loadCerts = function () { */ Docker.prototype.killSwarmContainer = function (cb) { var self = this; - log.info(self.logData, 'Docker.prototype.killSwarmContainer'); + log.info(self._logData, 'Docker.prototype.killSwarmContainer'); - self.client.getContainer('swarm').kill(function (err) { + self._client.getContainer('swarm').kill(function (err) { if (err) { - log.error(put({ err: err }, self.logData), 'killSwarmContainer error killing container'); + log.error(put({ err: err }, self._logData), 'killSwarmContainer error killing container'); return cb(err); } - log.trace(self.logData, 'killSwarmContainer killing container success'); + log.trace(self._logData, 'killSwarmContainer killing container success'); cb(null); }); }; diff --git a/test/functional/workers/on-dock-unhealthy.js b/test/functional/workers/on-dock-unhealthy.js index d0e5d46..213bdd8 100644 --- a/test/functional/workers/on-dock-unhealthy.js +++ b/test/functional/workers/on-dock-unhealthy.js @@ -62,7 +62,7 @@ var testRedis = redis.createClient( process.env.REDIS_IPADDRESS); -describe('on-dock-unhealthy functional test', function () { +describe('lib/workers/on-dock-unhealthy functional test', function () { var testHost = 'http://10.20.1.26:4242'; var testGihubId = 2194285; diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js index 2881688..acd7c69 100644 --- a/test/unit/models/consul.js +++ b/test/unit/models/consul.js @@ -16,7 +16,7 @@ var TaskError = require('ponos').TaskError; var Consul = require('../../../lib/models/consul.js'); -describe('consul unit test', function () { +describe('lib/models/consul unit test', function () { describe('waitForDockRemoved', function () { var dockerUrl = 'http://11.17.38.11:4242'; @@ -31,15 +31,18 @@ describe('consul unit test', function () { }); it('should cb TaskError if get errd', function (done) { - Consul._client.kv.get.yieldsAsync(new Error('starcraft')); + var error = new Error('starcraft'); + + Consul._client.kv.get.yieldsAsync(error); Consul.waitForDockRemoved(dockerUrl, function (err) { - expect(err).to.exist(); + expect(err).to.equal(error); done(); }); }); it('should cb TaskError if result exist', function (done) { + // returning non falsey means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, {some: 'stuff'}); Consul.waitForDockRemoved(dockerUrl, function (err) { @@ -49,6 +52,7 @@ describe('consul unit test', function () { }); it('should cb with no err if result does not exist', function (done) { + // returning null means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, null); Consul.waitForDockRemoved(dockerUrl, function (err) { diff --git a/test/unit/models/docker.js b/test/unit/models/docker.js index ac97e4d..4f8bd0d 100644 --- a/test/unit/models/docker.js +++ b/test/unit/models/docker.js @@ -15,11 +15,11 @@ var sinon = require('sinon'); var Docker = require('../../../lib/models/docker.js'); -describe('docker unit test', function () { +describe('lib/models/docker unit test', function () { describe('constructor', function () { it('should create Docker', function (done) { var docker = new Docker('http://10.0.0.1:4242'); - expect(docker.client).to.be.an.instanceof(Dockerode); + expect(docker._client).to.be.an.instanceof(Dockerode); done(); }); }); // end constructor @@ -57,7 +57,7 @@ describe('docker unit test', function () { describe('killSwarmContainer', function () { beforeEach(function (done) { - docker.client = { + docker._client = { getContainer: sinon.stub().returnsThis(), kill: sinon.stub() }; @@ -65,26 +65,27 @@ describe('docker unit test', function () { }); it('should cb error if kill failed', function (done) { - docker.client.kill.yieldsAsync(new Error('iceberg')); + var error = new Error('iceberg'); + docker._client.kill.yieldsAsync(error); docker.killSwarmContainer(function (err) { - expect(err).to.exist(); + expect(err).to.equal(error); done(); }); }); it('should cb on success', function (done) { - docker.client.kill.yieldsAsync(); + docker._client.kill.yieldsAsync(); docker.killSwarmContainer(function (err) { expect(err).to.not.exist(); - sinon.assert.calledOnce(docker.client.getContainer); + sinon.assert.calledOnce(docker._client.getContainer); sinon.assert.calledWith( - docker.client.getContainer, + docker._client.getContainer, 'swarm' ); - sinon.assert.calledOnce(docker.client.kill); + sinon.assert.calledOnce(docker._client.kill); done(); }); diff --git a/test/unit/models/events.js b/test/unit/models/events.js index d144d39..79bbf22 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -39,7 +39,7 @@ function dataExpectNone (data) { expect(data.length).to.equal(0); } -lab.experiment('events.js unit test', function () { +lab.experiment('lib/models/events.js unit test', function () { lab.beforeEach(function (done) { redis.connect(); redis.client.flushall(done); @@ -584,10 +584,11 @@ lab.experiment('events.js unit test', function () { }); lab.test('should cb err if waitForDockRemoved failed', function (done) { - Consul.waitForDockRemoved.yieldsAsync(new Error('blue')); + var error = new Error('blue'); + Consul.waitForDockRemoved.yieldsAsync(error); events.handleWaitForDockRemoved({}, function (err) { - expect(err).to.exist(); + expect(err).to.equal(error); done(); }); }); diff --git a/test/unit/models/redis.js b/test/unit/models/redis.js index e72a7aa..52084a9 100644 --- a/test/unit/models/redis.js +++ b/test/unit/models/redis.js @@ -14,7 +14,7 @@ var redis = require('redis'); var Redis = require('../../../lib/models/redis.js'); -describe('redis.js unit test', function () { +describe('lib/models/redis.js unit test', function () { describe('connect', function () { beforeEach(function (done) { sinon.stub(redis, 'createClient'); diff --git a/test/unit/models/worker-server.js b/test/unit/models/worker-server.js index 0726353..eade417 100644 --- a/test/unit/models/worker-server.js +++ b/test/unit/models/worker-server.js @@ -16,7 +16,7 @@ var ponos = require('ponos'); var RabbitMQ = require('../../../lib/rabbitmq.js'); var WorkerServer = require('../../../lib/models/worker-server.js'); -describe('WorkerServer unit test', function () { +describe('lib/models/worker-server unit test', function () { describe('listen', function () { beforeEach(function (done) { sinon.stub(RabbitMQ, 'getSubscriber'); diff --git a/test/unit/workers/container.life-cycle.died.js b/test/unit/workers/container.life-cycle.died.js index 9a22db8..59b104d 100644 --- a/test/unit/workers/container.life-cycle.died.js +++ b/test/unit/workers/container.life-cycle.died.js @@ -1,29 +1,22 @@ 'use strict'; require('loadenv')(); -var Promise = require('bluebird') var Lab = require('lab'); var lab = exports.lab = Lab.script(); var describe = lab.describe; var it = lab.it; -var afterEach = lab.afterEach; -var beforeEach = lab.beforeEach; var Code = require('code'); var expect = Code.expect; - var sinon = require('sinon'); var ponos = require('ponos'); var TaskFatalError = ponos.TaskFatalError; -var ErrorCat = require('error-cat'); -var RabbitMQ = require('../../../lib/rabbitmq.js'); var Events = require('../../../lib/models/events.js'); var dockData = require('../../../lib/models/dockData.js'); var containerLifeCycleDied = require('../../../lib/workers/container.life-cycle.died.js'); describe('container.life-cycle.died.js unit test', function () { - it('should throw error if invalid from', function (done) { sinon.stub(Events, '_hasValidFrom').returns(false); containerLifeCycleDied({}) @@ -72,9 +65,7 @@ describe('container.life-cycle.died.js unit test', function () { dockData.incKey.restore(); done(); }) - .catch(function (err) { - throw new Error('Should not happen'); - }); + .catch(done); }); it('should increment counter if build container', function (done) { @@ -100,8 +91,6 @@ describe('container.life-cycle.died.js unit test', function () { dockData.incKey.restore(); done(); }) - .catch(function (err) { - throw new Error('Should not happen'); - }); + .catch(done); }); }); // end container.life-cycle.died unit test diff --git a/test/unit/workers/dock.wait-for-removal.js b/test/unit/workers/dock.wait-for-removal.js index 5b4c0a9..1833c71 100644 --- a/test/unit/workers/dock.wait-for-removal.js +++ b/test/unit/workers/dock.wait-for-removal.js @@ -16,7 +16,7 @@ var TaskFatalError = require('ponos').TaskFatalError; var Events = require('../../../lib/models/events.js'); var waitForDockRemovedWorker = require('../../../lib/workers/dock.wait-for-removal.js'); -describe('dock.wait-for-removal unit test', function () { +describe('lib/workers/dock.wait-for-removal unit test', function () { describe('run', function () { beforeEach(function (done) { sinon.stub(Events, 'handleWaitForDockRemovedAsync'); @@ -29,7 +29,8 @@ describe('dock.wait-for-removal unit test', function () { }); it('should throw error if handleWaitForDockRemovedAsync failed', function (done) { - Events.handleWaitForDockRemovedAsync.throws(new Error('test')); + var error = new Error('test'); + Events.handleWaitForDockRemovedAsync.throws(error); waitForDockRemovedWorker({ dockerUrl: '10.0.0.1:4224', }) @@ -37,7 +38,7 @@ describe('dock.wait-for-removal unit test', function () { throw new Error('should have thrown'); }) .catch(function (err) { - expect(err).to.be.instanceOf(Error); + expect(err).to.equal(error); done(); }); }); diff --git a/test/unit/workers/docker.events-stream.connected.js b/test/unit/workers/docker.events-stream.connected.js index ca6663c..9af89d8 100644 --- a/test/unit/workers/docker.events-stream.connected.js +++ b/test/unit/workers/docker.events-stream.connected.js @@ -1,29 +1,22 @@ 'use strict'; require('loadenv')(); -var Promise = require('bluebird') var Lab = require('lab'); var lab = exports.lab = Lab.script(); var describe = lab.describe; var it = lab.it; -var afterEach = lab.afterEach; -var beforeEach = lab.beforeEach; var Code = require('code'); var expect = Code.expect; - var sinon = require('sinon'); var ponos = require('ponos'); var TaskFatalError = ponos.TaskFatalError; -var ErrorCat = require('error-cat'); -var RabbitMQ = require('../../../lib/rabbitmq.js'); var Events = require('../../../lib/models/events.js'); var dockData = require('../../../lib/models/dockData.js'); var dockerEventsStreamConnected = require('../../../lib/workers/docker.events-stream.connected.js'); -describe('docker.events-stream.connected.js unit test', function () { - +describe('lib/workers/docker.events-stream.connected.js unit test', function () { it('should throw error if invalid host', function (done) { sinon.stub(Events, '_hasValidHost').returns(false); dockerEventsStreamConnected({}) @@ -55,8 +48,6 @@ describe('docker.events-stream.connected.js unit test', function () { dockData.addHost.restore(); done(); }) - .catch(function (err) { - throw new Error('Should not happen'); - }); + .catch(done); }); }); // end docker.events-stream.connected unit test diff --git a/test/unit/workers/docker.events-stream.disconnected.js b/test/unit/workers/docker.events-stream.disconnected.js index b4a24fd..1c1327b 100644 --- a/test/unit/workers/docker.events-stream.disconnected.js +++ b/test/unit/workers/docker.events-stream.disconnected.js @@ -1,28 +1,22 @@ 'use strict'; require('loadenv')(); -var Promise = require('bluebird') var Lab = require('lab'); var lab = exports.lab = Lab.script(); var describe = lab.describe; var it = lab.it; -var afterEach = lab.afterEach; -var beforeEach = lab.beforeEach; var Code = require('code'); var expect = Code.expect; - var sinon = require('sinon'); var ponos = require('ponos'); var TaskFatalError = ponos.TaskFatalError; -var ErrorCat = require('error-cat'); -var RabbitMQ = require('../../../lib/rabbitmq.js'); var Events = require('../../../lib/models/events.js'); var dockData = require('../../../lib/models/dockData.js'); var dockerEventsStreamDisconnected = require('../../../lib/workers/docker.events-stream.disconnected.js'); -describe('docker.events-stream.disconnected.js unit test', function () { +describe('lib/workers/docker.events-stream.disconnected.js unit test', function () { it('should throw error if invalid host', function (done) { sinon.stub(Events, '_hasValidHost').returns(false); @@ -54,8 +48,6 @@ describe('docker.events-stream.disconnected.js unit test', function () { dockData.deleteHost.restore(); done(); }) - .catch(function (err) { - throw new Error('Should not happen'); - }); + .catch(done); }); }); // end docker.events-stream.disconnected unit test diff --git a/test/unit/workers/on-dock-unhealthy.js b/test/unit/workers/on-dock-unhealthy.js index e0b4d9c..98be252 100644 --- a/test/unit/workers/on-dock-unhealthy.js +++ b/test/unit/workers/on-dock-unhealthy.js @@ -16,14 +16,13 @@ var sinon = require('sinon'); var ponos = require('ponos'); var TaskFatalError = ponos.TaskFatalError; var TaskError = ponos.TaskError; -var Worker = require('../../../lib/workers/on-dock-unhealthy.js'); var dockData = require('../../../lib/models/dockData.js'); var rabbitMQ = require('../../../lib/rabbitmq.js'); var Events = require('../../../lib/models/events.js'); var Docker = require('../../../lib/models/docker.js'); var onDockUnhealthy = require('../../../lib/workers/on-dock-unhealthy.js'); -describe('on-dock-unhealthy unit test', function () { +describe('lib/workers/on-dock-unhealthy unit test', function () { it('should throw error if invalid host', function (done) { sinon.spy(Events, '_hasValidHost'); onDockUnhealthy({}) From a2c8d06868484317a093290f56ef55edf82108f8 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sun, 6 Dec 2015 17:22:58 -0800 Subject: [PATCH 21/26] waitForDockRemoved -> ensureDockRemoved --- lib/models/consul.js | 6 +++--- lib/models/events.js | 8 ++++---- lib/workers/dock.wait-for-removal.js | 2 +- test/unit/models/consul.js | 10 +++++----- test/unit/models/events.js | 22 +++++++++++----------- test/unit/workers/dock.wait-for-removal.js | 18 +++++++++--------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index da9f856..364e145 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -32,10 +32,10 @@ Consul._client = require('consul')({ * @param {String} dockerUrl docker host to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ -Consul.waitForDockRemoved = function (dockerUrl, cb) { +Consul.ensureDockRemoved = function (dockerUrl, cb) { var host = url.parse(dockerUrl).host; var logData = { host: host }; - log.info(logData, 'Consul.prototype.waitForDockRemoved'); + log.info(logData, 'Consul.prototype.ensureDockRemoved'); Consul._client.kv.get('swarm/docker/swarm/nodes/' + host, function(err, result) { if (err) { return cb(err); } // if we have a result that means the key still exist, cb with error @@ -44,7 +44,7 @@ Consul.waitForDockRemoved = function (dockerUrl, cb) { return cb(taskErr); } - log.trace(logData, 'waitForDockRemoved dock as been removed'); + log.trace(logData, 'ensureDockRemoved dock as been removed'); return cb(null); }); }; diff --git a/lib/models/events.js b/lib/models/events.js index 41394aa..a376c3f 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -111,12 +111,12 @@ Events.handleUnhealthy = function (data, cb) { * @param {String} data.dockerUrl url to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ -Events.handleWaitForDockRemoved = function (data, cb) { - log.info({ data: data }, 'Events.handleWaitForDockRemoved'); - Consul.waitForDockRemoved(data.dockerUrl, function (err) { +Events.handleensureDockRemoved = function (data, cb) { + log.info({ data: data }, 'Events.handleensureDockRemoved'); + Consul.ensureDockRemoved(data.dockerUrl, function (err) { if (err) { return cb(err); } - log.trace({ data: data }, 'handleWaitForDockRemoved publishing dock.removed'); + log.trace({ data: data }, 'handleensureDockRemoved publishing dock.removed'); rabbitMQ.getPublisher().publish('dock.removed', { host: data.dockerUrl }); diff --git a/lib/workers/dock.wait-for-removal.js b/lib/workers/dock.wait-for-removal.js index 9c41f25..55a52bc 100644 --- a/lib/workers/dock.wait-for-removal.js +++ b/lib/workers/dock.wait-for-removal.js @@ -19,7 +19,7 @@ module.exports = function (job) { } }) .then(function () { - return Events.handleWaitForDockRemovedAsync(job); + return Events.handleensureDockRemovedAsync(job); }) .catch(function (err) { log.error({ err: err }, 'dock.wait-for-removal error'); diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js index acd7c69..bece382 100644 --- a/test/unit/models/consul.js +++ b/test/unit/models/consul.js @@ -17,7 +17,7 @@ var TaskError = require('ponos').TaskError; var Consul = require('../../../lib/models/consul.js'); describe('lib/models/consul unit test', function () { - describe('waitForDockRemoved', function () { + describe('ensureDockRemoved', function () { var dockerUrl = 'http://11.17.38.11:4242'; beforeEach(function (done) { @@ -35,7 +35,7 @@ describe('lib/models/consul unit test', function () { Consul._client.kv.get.yieldsAsync(error); - Consul.waitForDockRemoved(dockerUrl, function (err) { + Consul.ensureDockRemoved(dockerUrl, function (err) { expect(err).to.equal(error); done(); }); @@ -45,7 +45,7 @@ describe('lib/models/consul unit test', function () { // returning non falsey means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, {some: 'stuff'}); - Consul.waitForDockRemoved(dockerUrl, function (err) { + Consul.ensureDockRemoved(dockerUrl, function (err) { expect(err).to.be.an.instanceof(TaskError); done(); }); @@ -55,7 +55,7 @@ describe('lib/models/consul unit test', function () { // returning null means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, null); - Consul.waitForDockRemoved(dockerUrl, function (err) { + Consul.ensureDockRemoved(dockerUrl, function (err) { expect(err).to.not.exist(); sinon.assert.calledOnce(Consul._client.kv.get); @@ -67,5 +67,5 @@ describe('lib/models/consul unit test', function () { done(); }); }); - }); // end waitForDockRemoved + }); // end ensureDockRemoved }); \ No newline at end of file diff --git a/test/unit/models/events.js b/test/unit/models/events.js index 79bbf22..f6a6a5f 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -566,10 +566,10 @@ lab.experiment('lib/models/events.js unit test', function () { }); // handleDockDown }); // deamon - lab.experiment('handleWaitForDockRemoved', function () { + lab.experiment('handleensureDockRemoved', function () { var publishStub; beforeEach(function (done) { - sinon.stub(Consul, 'waitForDockRemoved'); + sinon.stub(Consul, 'ensureDockRemoved'); publishStub = sinon.stub(); sinon.stub(RabbitMQ, 'getPublisher').returns({ publish: publishStub @@ -578,16 +578,16 @@ lab.experiment('lib/models/events.js unit test', function () { }); afterEach(function (done) { - Consul.waitForDockRemoved.restore(); + Consul.ensureDockRemoved.restore(); RabbitMQ.getPublisher.restore(); done(); }); - lab.test('should cb err if waitForDockRemoved failed', function (done) { + lab.test('should cb err if ensureDockRemoved failed', function (done) { var error = new Error('blue'); - Consul.waitForDockRemoved.yieldsAsync(error); + Consul.ensureDockRemoved.yieldsAsync(error); - events.handleWaitForDockRemoved({}, function (err) { + events.handleensureDockRemoved({}, function (err) { expect(err).to.equal(error); done(); }); @@ -595,16 +595,16 @@ lab.experiment('lib/models/events.js unit test', function () { lab.test('should publish dock.removed', function (done) { var dockerUrl = 'http://10.0.102.2:4242'; - Consul.waitForDockRemoved.yieldsAsync(null); + Consul.ensureDockRemoved.yieldsAsync(null); - events.handleWaitForDockRemoved({ + events.handleensureDockRemoved({ dockerUrl: dockerUrl }, function (err) { expect(err).to.not.exist(); - sinon.assert.calledOnce(Consul.waitForDockRemoved); + sinon.assert.calledOnce(Consul.ensureDockRemoved); sinon.assert.calledWith( - Consul.waitForDockRemoved, + Consul.ensureDockRemoved, dockerUrl ); @@ -618,5 +618,5 @@ lab.experiment('lib/models/events.js unit test', function () { done(); }); }); - }); // end handleWaitForDockRemoved + }); // end handleensureDockRemoved }); // docker events diff --git a/test/unit/workers/dock.wait-for-removal.js b/test/unit/workers/dock.wait-for-removal.js index 1833c71..8d0218f 100644 --- a/test/unit/workers/dock.wait-for-removal.js +++ b/test/unit/workers/dock.wait-for-removal.js @@ -14,24 +14,24 @@ var sinon = require('sinon'); var TaskFatalError = require('ponos').TaskFatalError; var Events = require('../../../lib/models/events.js'); -var waitForDockRemovedWorker = require('../../../lib/workers/dock.wait-for-removal.js'); +var ensureDockRemovedWorker = require('../../../lib/workers/dock.wait-for-removal.js'); describe('lib/workers/dock.wait-for-removal unit test', function () { describe('run', function () { beforeEach(function (done) { - sinon.stub(Events, 'handleWaitForDockRemovedAsync'); + sinon.stub(Events, 'handleensureDockRemovedAsync'); done(); }); afterEach(function (done) { - Events.handleWaitForDockRemovedAsync.restore(); + Events.handleensureDockRemovedAsync.restore(); done(); }); - it('should throw error if handleWaitForDockRemovedAsync failed', function (done) { + it('should throw error if handleensureDockRemovedAsync failed', function (done) { var error = new Error('test'); - Events.handleWaitForDockRemovedAsync.throws(error); - waitForDockRemovedWorker({ + Events.handleensureDockRemovedAsync.throws(error); + ensureDockRemovedWorker({ dockerUrl: '10.0.0.1:4224', }) .then(function () { @@ -44,7 +44,7 @@ describe('lib/workers/dock.wait-for-removal unit test', function () { }); it('should throw missing dockerUrl', function (done) { - waitForDockRemovedWorker({}) + ensureDockRemovedWorker({}) .then(function () { throw new Error('should have thrown'); }) @@ -55,8 +55,8 @@ describe('lib/workers/dock.wait-for-removal unit test', function () { }); it('should be fine if no errors', function (done) { - Events.handleWaitForDockRemovedAsync.returns(); - waitForDockRemovedWorker({ + Events.handleensureDockRemovedAsync.returns(); + ensureDockRemovedWorker({ dockerUrl: '10.0.0.1:4224' }) .then(done) From 3e7f20b02d44a7dba88cd493b6957b61bcbf9a4e Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sun, 6 Dec 2015 17:25:30 -0800 Subject: [PATCH 22/26] make swarm a env SWARM_CONTAINER_NAME --- lib/models/docker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/docker.js b/lib/models/docker.js index 5b2c772..6dc3b9c 100644 --- a/lib/models/docker.js +++ b/lib/models/docker.js @@ -62,7 +62,7 @@ Docker.prototype.killSwarmContainer = function (cb) { var self = this; log.info(self._logData, 'Docker.prototype.killSwarmContainer'); - self._client.getContainer('swarm').kill(function (err) { + self._client.getContainer(process.env.SWARM_CONTAINER_NAME).kill(function (err) { if (err) { log.error(put({ err: err }, self._logData), 'killSwarmContainer error killing container'); return cb(err); From 8468aa364a0380329eac44825c3f3865f8a9b5b5 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sun, 6 Dec 2015 17:25:39 -0800 Subject: [PATCH 23/26] add swarm env to test --- configs/.env.test | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/.env.test b/configs/.env.test index 36fa68d..551b2bb 100644 --- a/configs/.env.test +++ b/configs/.env.test @@ -8,3 +8,4 @@ RABBITMQ_PASSWORD=guest RABBITMQ_USERNAME=guest REDIS_IPADDRESS=127.0.0.1 REDIS_PORT=6379 +SWARM_CONTAINER_NAME=swarm From b3bac2a1149f0a0f2c2d6d967e7bb8e322815ed6 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Sun, 6 Dec 2015 17:39:58 -0800 Subject: [PATCH 24/26] rename some stuff, throw error highter up --- lib/models/consul.js | 6 +++--- lib/models/events.js | 11 +++++++---- lib/workers/dock.wait-for-removal.js | 2 +- test/unit/models/consul.js | 2 +- test/unit/models/events.js | 14 +++++++------- test/unit/workers/dock.wait-for-removal.js | 10 +++++----- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index 364e145..f1757c7 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -6,7 +6,8 @@ require('loadenv')('mavis:env'); var url = require('url'); -var TaskError = require('ponos').TaskError; +var ErrorCat = require('error-cat'); +var error = new ErrorCat(); var log = require('../logger').child({ module: 'consul' }); @@ -40,8 +41,7 @@ Consul.ensureDockRemoved = function (dockerUrl, cb) { if (err) { return cb(err); } // if we have a result that means the key still exist, cb with error if (result) { - var taskErr = new TaskError('dock.wait-for-removal', 'dock still exists', result); - return cb(taskErr); + return cb(error.create(412, 'dock still exist', result)); } log.trace(logData, 'ensureDockRemoved dock as been removed'); diff --git a/lib/models/events.js b/lib/models/events.js index a376c3f..6f89fd0 100644 --- a/lib/models/events.js +++ b/lib/models/events.js @@ -111,12 +111,15 @@ Events.handleUnhealthy = function (data, cb) { * @param {String} data.dockerUrl url to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ -Events.handleensureDockRemoved = function (data, cb) { - log.info({ data: data }, 'Events.handleensureDockRemoved'); +Events.handleEnsureDockRemoved = function (data, cb) { + log.info({ data: data }, 'Events.handleEnsureDockRemoved'); Consul.ensureDockRemoved(data.dockerUrl, function (err) { - if (err) { return cb(err); } + if (err) { + var taskErr = new TaskError('dock.wait-for-removal', 'dock still exists', err); + return cb(taskErr); + } - log.trace({ data: data }, 'handleensureDockRemoved publishing dock.removed'); + log.trace({ data: data }, 'handleEnsureDockRemoved publishing dock.removed'); rabbitMQ.getPublisher().publish('dock.removed', { host: data.dockerUrl }); diff --git a/lib/workers/dock.wait-for-removal.js b/lib/workers/dock.wait-for-removal.js index 55a52bc..a90077e 100644 --- a/lib/workers/dock.wait-for-removal.js +++ b/lib/workers/dock.wait-for-removal.js @@ -19,7 +19,7 @@ module.exports = function (job) { } }) .then(function () { - return Events.handleensureDockRemovedAsync(job); + return Events.handleEnsureDockRemovedAsync(job); }) .catch(function (err) { log.error({ err: err }, 'dock.wait-for-removal error'); diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js index bece382..167afc7 100644 --- a/test/unit/models/consul.js +++ b/test/unit/models/consul.js @@ -46,7 +46,7 @@ describe('lib/models/consul unit test', function () { Consul._client.kv.get.yieldsAsync(null, {some: 'stuff'}); Consul.ensureDockRemoved(dockerUrl, function (err) { - expect(err).to.be.an.instanceof(TaskError); + expect(err).to.exist(Error); done(); }); }); diff --git a/test/unit/models/events.js b/test/unit/models/events.js index f6a6a5f..2f6d060 100644 --- a/test/unit/models/events.js +++ b/test/unit/models/events.js @@ -17,6 +17,7 @@ var Consul = require('../../../lib/models/consul.js'); var async = require('async'); var sinon = require('sinon'); +var TaskError = require('ponos').TaskError; var host = 'http://0.0.0.0:4242'; @@ -566,7 +567,7 @@ lab.experiment('lib/models/events.js unit test', function () { }); // handleDockDown }); // deamon - lab.experiment('handleensureDockRemoved', function () { + lab.experiment('handleEnsureDockRemoved', function () { var publishStub; beforeEach(function (done) { sinon.stub(Consul, 'ensureDockRemoved'); @@ -584,11 +585,10 @@ lab.experiment('lib/models/events.js unit test', function () { }); lab.test('should cb err if ensureDockRemoved failed', function (done) { - var error = new Error('blue'); - Consul.ensureDockRemoved.yieldsAsync(error); + Consul.ensureDockRemoved.yieldsAsync(new Error('dock not removed')); - events.handleensureDockRemoved({}, function (err) { - expect(err).to.equal(error); + events.handleEnsureDockRemoved({}, function (err) { + expect(err).to.be.an.instanceOf(TaskError); done(); }); }); @@ -597,7 +597,7 @@ lab.experiment('lib/models/events.js unit test', function () { var dockerUrl = 'http://10.0.102.2:4242'; Consul.ensureDockRemoved.yieldsAsync(null); - events.handleensureDockRemoved({ + events.handleEnsureDockRemoved({ dockerUrl: dockerUrl }, function (err) { expect(err).to.not.exist(); @@ -618,5 +618,5 @@ lab.experiment('lib/models/events.js unit test', function () { done(); }); }); - }); // end handleensureDockRemoved + }); // end handleEnsureDockRemoved }); // docker events diff --git a/test/unit/workers/dock.wait-for-removal.js b/test/unit/workers/dock.wait-for-removal.js index 8d0218f..52cfe76 100644 --- a/test/unit/workers/dock.wait-for-removal.js +++ b/test/unit/workers/dock.wait-for-removal.js @@ -19,18 +19,18 @@ var ensureDockRemovedWorker = require('../../../lib/workers/dock.wait-for-remova describe('lib/workers/dock.wait-for-removal unit test', function () { describe('run', function () { beforeEach(function (done) { - sinon.stub(Events, 'handleensureDockRemovedAsync'); + sinon.stub(Events, 'handleEnsureDockRemovedAsync'); done(); }); afterEach(function (done) { - Events.handleensureDockRemovedAsync.restore(); + Events.handleEnsureDockRemovedAsync.restore(); done(); }); - it('should throw error if handleensureDockRemovedAsync failed', function (done) { + it('should throw error if handleEnsureDockRemovedAsync failed', function (done) { var error = new Error('test'); - Events.handleensureDockRemovedAsync.throws(error); + Events.handleEnsureDockRemovedAsync.throws(error); ensureDockRemovedWorker({ dockerUrl: '10.0.0.1:4224', }) @@ -55,7 +55,7 @@ describe('lib/workers/dock.wait-for-removal unit test', function () { }); it('should be fine if no errors', function (done) { - Events.handleensureDockRemovedAsync.returns(); + Events.handleEnsureDockRemovedAsync.returns(); ensureDockRemovedWorker({ dockerUrl: '10.0.0.1:4224' }) From 3f9e11bf0df0043eb612197d984acd1129f88964 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Mon, 7 Dec 2015 15:24:23 -0800 Subject: [PATCH 25/26] fix test --- test/unit/models/consul.js | 6 +- test/unit/models/docker.js | 8 +- test/unit/workers/dock.wait-for-removal.js | 14 +-- test/unit/workers/on-dock-unhealthy.js | 122 ++++++++++++--------- 4 files changed, 80 insertions(+), 70 deletions(-) diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js index 167afc7..af846f0 100644 --- a/test/unit/models/consul.js +++ b/test/unit/models/consul.js @@ -30,7 +30,7 @@ describe('lib/models/consul unit test', function () { done(); }); - it('should cb TaskError if get errd', function (done) { + it('should cb with error from Consul._client.kv.get', function (done) { var error = new Error('starcraft'); Consul._client.kv.get.yieldsAsync(error); @@ -41,12 +41,12 @@ describe('lib/models/consul unit test', function () { }); }); - it('should cb TaskError if result exist', function (done) { + it('should cb an err if result exists', function (done) { // returning non falsey means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, {some: 'stuff'}); Consul.ensureDockRemoved(dockerUrl, function (err) { - expect(err).to.exist(Error); + expect(err).to.exist(); done(); }); }); diff --git a/test/unit/models/docker.js b/test/unit/models/docker.js index 4f8bd0d..3c1947c 100644 --- a/test/unit/models/docker.js +++ b/test/unit/models/docker.js @@ -29,18 +29,14 @@ describe('lib/models/docker unit test', function () { it('should throw if missing certs', function (done) { process.env.DOCKER_CERT_PATH = 'fake/path'; - expect(function () { - Docker.loadCerts(); - }).to.throw(); + expect(Docker.loadCerts).to.throw(); done(); }); it('should load certs', function (done) { process.env.DOCKER_CERT_PATH = './test/fixtures/certs'; - expect(function () { - Docker.loadCerts(); - }).to.not.throw(); + expect(Docker.loadCerts).to.not.throw(); done(); }); }); // end loadCerts diff --git a/test/unit/workers/dock.wait-for-removal.js b/test/unit/workers/dock.wait-for-removal.js index 52cfe76..0402888 100644 --- a/test/unit/workers/dock.wait-for-removal.js +++ b/test/unit/workers/dock.wait-for-removal.js @@ -45,13 +45,13 @@ describe('lib/workers/dock.wait-for-removal unit test', function () { it('should throw missing dockerUrl', function (done) { ensureDockRemovedWorker({}) - .then(function () { - throw new Error('should have thrown'); - }) - .catch(function (err) { - expect(err).to.be.instanceOf(TaskFatalError); - done(); - }); + .then(function () { + throw new Error('should have thrown'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(TaskFatalError); + done(); + }); }); it('should be fine if no errors', function (done) { diff --git a/test/unit/workers/on-dock-unhealthy.js b/test/unit/workers/on-dock-unhealthy.js index 98be252..1681936 100644 --- a/test/unit/workers/on-dock-unhealthy.js +++ b/test/unit/workers/on-dock-unhealthy.js @@ -23,64 +23,77 @@ var Docker = require('../../../lib/models/docker.js'); var onDockUnhealthy = require('../../../lib/workers/on-dock-unhealthy.js'); describe('lib/workers/on-dock-unhealthy unit test', function () { - it('should throw error if invalid host', function (done) { - sinon.spy(Events, '_hasValidHost'); - onDockUnhealthy({}) + describe('failed', function () { + beforeEach(function (done) { + sinon.spy(Events, '_hasValidHost'); + sinon.stub(dockData, 'deleteHost') + sinon.stub(Docker.prototype, 'killSwarmContainer').yieldsAsync(null); + done(); + }); + + afterEach(function (done) { + Events._hasValidHost.restore(); + dockData.deleteHost.restore(); + Docker.prototype.killSwarmContainer.restore(); + done(); + }); + + it('should throw error if invalid host', function (done) { + onDockUnhealthy({}) + .then(function () { + throw new Error('Should not happen'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(TaskFatalError); + sinon.assert.calledOnce(Events._hasValidHost); + done(); + }); + }); + + it('should throw error delete host failed', function (done) { + dockData.deleteHost.yieldsAsync(new Error('Redis error')); + onDockUnhealthy({ + host: 'http://10.12.12.11:4242', + }) .then(function () { throw new Error('Should not happen'); }) .catch(function (err) { - expect(err).to.be.instanceOf(TaskFatalError); - expect(Events._hasValidHost.calledOnce).to.be.true(); - Events._hasValidHost.restore(); + expect(err).to.be.instanceOf(TaskError); + sinon.assert.calledOnce(Events._hasValidHost); + sinon.assert.calledOnce(dockData.deleteHost); + sinon.assert.calledWith( + dockData.deleteHost, + 'http://10.12.12.11:4242' + ); done(); }); - }); - - it('should throw error delete host failed', function (done) { - sinon.spy(Events, '_hasValidHost'); - sinon.stub(dockData, 'deleteHost').yieldsAsync(new Error('Redis error')); - onDockUnhealthy({ - host: 'http://10.12.12.11:4242', - }) - .then(function () { - throw new Error('Should not happen'); - }) - .catch(function (err) { - expect(err).to.be.instanceOf(TaskError); - expect(Events._hasValidHost.calledOnce).to.be.true(); - expect(dockData.deleteHost.called).to.be.true(); - expect(dockData.deleteHost.getCall(0).args[0]).to.equal('http://10.12.12.11:4242'); - dockData.deleteHost.restore(); - Events._hasValidHost.restore(); - done(); }); - }); - it('should throw if killingSwarmContainer failed', function (done) { - var dockerUrl = 'http://10.12.12.11:4242'; - - sinon.spy(Events, '_hasValidHost'); - sinon.stub(dockData, 'deleteHost').yieldsAsync(null); - sinon.stub(Docker.prototype, 'killSwarmContainer').yieldsAsync(new Error('Docker Error')); - - onDockUnhealthy({ - host: dockerUrl, - }) - .then(function () { - throw new Error('Should not happen'); - }) - .catch(function (err) { - expect(err).to.be.instanceOf(TaskError); - expect(Events._hasValidHost.calledOnce).to.be.true(); - expect(dockData.deleteHost.called).to.be.true(); - expect(dockData.deleteHost.getCall(0).args[0]).to.equal(dockerUrl); - dockData.deleteHost.restore(); - Events._hasValidHost.restore(); - Docker.prototype.killSwarmContainer.restore(); - done(); + it('should throw if killingSwarmContainer failed', function (done) { + var dockerUrl = 'http://10.12.12.11:4242'; + + dockData.deleteHost.yieldsAsync(null); + Docker.prototype.killSwarmContainer.yieldsAsync(new Error('Docker Error')); + + onDockUnhealthy({ + host: dockerUrl, + }) + .then(function () { + throw new Error('Should not happen'); + }) + .catch(function (err) { + expect(err).to.be.instanceOf(TaskError); + sinon.assert.calledOnce(Events._hasValidHost); + sinon.assert.calledOnce(dockData.deleteHost); + sinon.assert.calledWith( + dockData.deleteHost, + dockerUrl + ); + done(); + }); }); - }); + }); // end failed describe('success', function () { beforeEach(function (done) { @@ -110,19 +123,20 @@ describe('lib/workers/on-dock-unhealthy unit test', function () { githubId: 12312 }) .then(function () { - expect(Events._hasValidHost.calledOnce).to.be.true(); + sinon.assert.calledOnce(Events._hasValidHost); - expect(dockData.deleteHost.called).to.be.true(); - expect(dockData.deleteHost.getCall(0).args[0]).to.equal(dockerUrl); + sinon.assert.calledOnce(dockData.deleteHost); + sinon.assert.calledWith( + dockData.deleteHost, + dockerUrl + ); sinon.assert.calledTwice(rabbitMQ._publisher.publish); - sinon.assert.calledWith( rabbitMQ._publisher.publish.getCall(0), 'cluster-instance-provision', sinon.match({ githubId: 12312 }) ); - sinon.assert.calledWith( rabbitMQ._publisher.publish.getCall(1), 'dock.wait-for-removal', From d2f8da853dcf1f93161fc12cce5f6cdeb47989a4 Mon Sep 17 00:00:00 2001 From: Anandkumar Patel Date: Mon, 7 Dec 2015 15:46:59 -0800 Subject: [PATCH 26/26] update words --- lib/models/consul.js | 4 ++-- test/unit/models/consul.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/models/consul.js b/lib/models/consul.js index f1757c7..1fd7ad9 100644 --- a/lib/models/consul.js +++ b/lib/models/consul.js @@ -28,8 +28,8 @@ Consul._client = require('consul')({ }); /** - * waits for dock to be removed from consul - * will cb with TaskError if dock still exist + * checks dock host key in consul to see if it exist. + * will cb with error if dock key still exist * @param {String} dockerUrl docker host to check for format: http://10.0.0.1:4242 * @param {Function} cb (err) */ diff --git a/test/unit/models/consul.js b/test/unit/models/consul.js index af846f0..7d6c18f 100644 --- a/test/unit/models/consul.js +++ b/test/unit/models/consul.js @@ -51,7 +51,8 @@ describe('lib/models/consul unit test', function () { }); }); - it('should cb with no err if result does not exist', function (done) { + // result and error will both be null when the key does not exist in consul + it('should cb with no err and null if result is null', function (done) { // returning null means the dock hasn't been removed Consul._client.kv.get.yieldsAsync(null, null);