Skip to content

Commit 5eb5c9c

Browse files
tjmehtatjmehta
committed
Merge pull request #426 from CodeNow/more-tests-for-the-events
More tests for the Docker Events
2 parents c86ad0a + 053de2b commit 5eb5c9c

14 files changed

Lines changed: 445 additions & 58 deletions

File tree

‎lib/middlewares/create-class-middleware.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ var keypather = require('keypather')();
44
var fno = require('fn-object');
55
var isString = require('101/is-string');
66
var isObject = require('101/is-object');
7+
var exists = require('101/exists');
78

89
module.exports = function (key, Model) {
910
return new ClassMiddleware(key, Model);
@@ -88,7 +89,8 @@ function valIsFunction (obj) {
8889
function replacePlaceholders (req) {
8990
return function (arg) {
9091
if (isString(arg)) {
91-
return keypather.get(req, arg) || arg;
92+
var value = keypather.get(req, arg);
93+
return exists(value) ? value : arg;
9294
}
9395
else if (Array.isArray(arg)) {
9496
return arg.map(replacePlaceholders(req));

‎lib/models/apis/docker.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ Docker.prototype.restartContainer = function (container, cb) {
397397

398398
/**
399399
* attempts to stop a running container.
400-
* if not stoped in passed in time, the process is kill 9'd
400+
* if not stopped in passed in time, the process is kill 9'd
401401
* @param container docker container or mongo container object
402402
* @param force Force stop a container. Ignores 'already stopped' error.
403403
* @param cb callback

‎lib/models/events/docker.js‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,23 @@ DockerEvents.prototype.listen = function (cb) {
2323
debug('listen', arguments);
2424
cb = cb || noop;
2525
if (this.closeHandler) {
26-
return cb(new Error('closing'));
26+
return cb(Boom.conflict('closing events listener is in progress'));
2727
}
2828
this.subscribeAll();
2929
cb();
3030
};
3131

3232
DockerEvents.prototype.close = function (cb) {
3333
debug('close', arguments, this.eventLockCount);
34+
cb = cb || noop;
3435
if (this.closeHandler) {
35-
return cb(new Error('already closing'));
36+
return cb(Boom.conflict('already closing events listener'));
3637
}
38+
var self = this;
3739
this.unsubscribeAll();
38-
this.closeHandler = cb || noop;
40+
this.closeHandler = cb;
3941
if (this.eventLockCount <= 0) {
4042
this.eventLockCount = 0; // to be safe
41-
var self = this;
4243
// prevent sync callback
4344
process.nextTick(function () {
4445
self.closeHandler();
@@ -114,7 +115,12 @@ DockerEvents.prototype.getEventLock = function (eventId, cb) {
114115

115116
DockerEvents.prototype.handleDie = function (data) {
116117
debug('handleDie', arguments);
117-
if (this.closeHandler) { return; } // this api is closing and will not handle any new events.
118+
// this api is closing and will not handle any new events.
119+
if (this.closeHandler) {
120+
// this debug statement is covered with unit test. Don't change/remove it.
121+
// see unit/docker-events.js
122+
return debug('events are stopping');
123+
}
118124
var self = this;
119125
var errDebug = {
120126
event: 'die',
@@ -127,7 +133,11 @@ DockerEvents.prototype.handleDie = function (data) {
127133
var containerId = data.id;
128134
activeApi.isMe(function (err, meIsActiveApi) {
129135
if (err) { return logErr(err); }
130-
if (!meIsActiveApi) { return debug('not active api'); }
136+
if (!meIsActiveApi) {
137+
// this debug statement is covered with unit test. Don't change/remove it.
138+
// see unit/docker-events.js
139+
return debug('not active api');
140+
}
131141
self.getEventLock(data.uuid, function (err, eventMutex) {
132142
if (err) { return logErr(err); }
133143
var userStoppedContainer = new UserStoppedContainer(containerId);

‎lib/models/events/index.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,30 @@
22
var domain = require('domain');
33

44
var error = require('error');
5+
var Boom = require('dat-middleware').Boom;
56

67
var d = domain.create();
78
d.on('error', error.log);
89

9-
function Events () {}
10+
11+
function Events () {
12+
this.connected = false;
13+
}
1014

1115
Events.prototype.listen = function (cb) {
16+
if (this.connected) {
17+
return cb(Boom.conflict('events were already started'));
18+
}
1219
var self = this;
1320
d.run(function () {
21+
self.connected = true;
1422
self.dockerEvents = require('./docker');
1523
self.dockerEvents.listen(cb);
1624
});
1725
};
1826

1927
Events.prototype.close = function (cb) {
28+
this.connected = false;
2029
if (this.dockerEvents) {
2130
this.dockerEvents.close(cb);
2231
}

‎lib/models/mongo/instance.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,10 @@ InstanceSchema.methods.populateDeps = function (seenNodes, cb) {
104104
InstanceSchema.statics.inspectAndUpdateByContainer = function (containerId, cb) {
105105
Instance.findByContainerId(containerId, function (err, instance) {
106106
if (err) { return cb(err); }
107-
if (!instance) { return cb(); } // no instance with container found.
107+
// no instance with container found.
108+
if (!instance) {
109+
return cb(Boom.notFound('Container not found'));
110+
}
108111
var container = instance.container;
109112
instance.inspectAndUpdate(container, container.dockerHost, cb);
110113
});

‎lib/models/redis/active-api.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ var debug = require('debug')('runnable-api:redis:active-api');
88

99

1010
function ActiveApi () {
11-
this.key = process.env.REDIS_NAMESPACE+'active-api';
11+
this.key = process.env.REDIS_NAMESPACE + 'active-api';
1212
}
1313

1414
require('util').inherits(ActiveApi, RedisKey);

‎test/bdd-deploy-instance.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('BDD - Create Build and Deploy Instance', function () {
3737
ctx.user = user;
3838
ctx.contextVersion = modelsArr[0];
3939
ctx.context = modelsArr[1];
40-
ctx.oldDockerContainer = ctx.instance.attrs.container.dockerContainer;
40+
ctx.oldDockerContainer = ctx.instance.attrs.containers[0].dockerContainer;
4141
done();
4242
});
4343
});
@@ -106,7 +106,7 @@ describe('BDD - Create Build and Deploy Instance', function () {
106106
}
107107
function tailInstance (newBuild, cb) {
108108
multi.tailInstance(ctx.user, ctx.instance, function (err) {
109-
expect(ctx.instance.attrs.container.dockerContainer).to.not.equal(ctx.oldDockerContainer);
109+
expect(ctx.instance.attrs.containers[0].dockerContainer).to.not.equal(ctx.oldDockerContainer);
110110
cb(err, newBuild);
111111
});
112112
}
@@ -180,7 +180,7 @@ describe('BDD - Create Build and Deploy Instance', function () {
180180
}
181181
function tailInstance (newBuild, cb) {
182182
multi.tailInstance(ctx.user, ctx.instance, function (err) {
183-
expect(ctx.instance.attrs.container.dockerContainer).to.not.equal(ctx.oldDockerContainer);
183+
expect(ctx.instance.attrs.containers[0].dockerContainer).to.not.equal(ctx.oldDockerContainer);
184184
cb(err, newBuild);
185185
});
186186
}
@@ -256,7 +256,7 @@ describe('BDD - Create Build and Deploy Instance', function () {
256256
}
257257
function tailInstance (newBuild, cb) {
258258
multi.tailInstance(ctx.user, ctx.instance, function (err) {
259-
expect(ctx.instance.attrs.container.dockerContainer).to.not.equal(ctx.oldDockerContainer);
259+
expect(ctx.instance.attrs.containers[0].dockerContainer).to.not.equal(ctx.oldDockerContainer);
260260
cb(err, newBuild);
261261
});
262262
}
@@ -356,7 +356,7 @@ describe('BDD - Create Build and Deploy Instance', function () {
356356
}
357357
function tailInstance (newBuild, cb) {
358358
multi.tailInstance(ctx.user, ctx.instance, function (err) {
359-
expect(ctx.instance.attrs.container.dockerContainer).to.not.equal(ctx.oldDockerContainer);
359+
expect(ctx.instance.attrs.containers[0].dockerContainer).to.not.equal(ctx.oldDockerContainer);
360360
cb(err, newBuild);
361361
});
362362
}

‎test/instances-id-actions-restart/put/index.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,11 @@ describe('PUT /instances/:id/actions/restart', { timeout: 500 }, function () {
139139
beforeEach(function (done) {
140140
extend(ctx.expected, {
141141
containers: exists,
142-
'container': exists,
143-
'container.ports': exists,
144-
'container.dockerHost': exists,
145-
'container.dockerContainer': exists,
146-
'container.inspect.State.Running': true
142+
'containers[0]': exists,
143+
'containers[0].ports': exists,
144+
'containers[0].dockerHost': exists,
145+
'containers[0].dockerContainer': exists,
146+
'containers[0].inspect.State.Running': true
147147
});
148148
ctx.expectAlreadyStarted = true;
149149
done();
@@ -155,10 +155,10 @@ describe('PUT /instances/:id/actions/restart', { timeout: 500 }, function () {
155155
beforeEach(function (done) {
156156
extend(ctx.expected, {
157157
containers: exists,
158-
'container': exists,
159-
'container.dockerHost': exists,
160-
'container.dockerContainer': exists,
161-
'container.inspect.State.Running': false
158+
'containers[0]': exists,
159+
'containers[0].dockerHost': exists,
160+
'containers[0].dockerContainer': exists,
161+
'containers[0].inspect.State.Running': false
162162
});
163163
ctx.originalStart = Docker.prototype.startContainer;
164164
Docker.prototype.startContainer = stopContainerRightAfterStart;
@@ -174,8 +174,8 @@ describe('PUT /instances/:id/actions/restart', { timeout: 500 }, function () {
174174
});
175175
describe('Container create error (Invalid dockerfile CMD)', function() {
176176
beforeEach(function (done) {
177-
ctx.expected['container.error.message'] = exists;
178-
ctx.expected['container.error.stack'] = exists;
177+
ctx.expected['containers[0].error.message'] = exists;
178+
ctx.expected['containers[0].error.stack'] = exists;
179179
ctx.expectNoContainerErr = true;
180180
ctx.originalCreateContainer = Dockerode.prototype.createContainer;
181181
Dockerode.prototype.createContainer = forceCreateContainerErr;
@@ -226,7 +226,7 @@ describe('PUT /instances/:id/actions/restart', { timeout: 500 }, function () {
226226
it('should restart an instance', function (done) {
227227
if (ctx.originalStart) { // restore docker back to normal - immediately exiting container will now start
228228
Docker.prototype.startContainer = ctx.originalStart;
229-
ctx.expected['container.inspect.State.Running'] = true;
229+
ctx.expected['containers[0].inspect.State.Running'] = true;
230230
}
231231
if (ctx.expectNoContainerErr) {
232232
ctx.instance.restart(expects.error(400, /not have a container/, done));

‎test/instances-id-actions-start/put/index.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@ describe('PUT /instances/:id/actions/start', { timeout: 500 }, function () {
187187
Dockerode.prototype.createContainer = ctx.originalCreateContainer;
188188
done();
189189
});
190-
191190
createInstanceAndRunTests(ctx);
192191
});
193192
});

‎test/instances-id-actions-stop/put/index.js‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,11 @@ describe('PUT /instances/:id/actions/stop', {timeout:1000}, function () {
140140
beforeEach(function (done) {
141141
extend(ctx.expected, {
142142
containers: exists,
143-
'container': exists,
144-
'container.ports': exists,
145-
'container.dockerHost': exists,
146-
'container.dockerContainer': exists,
147-
'container.inspect.State.Running': true
143+
'containers[0]': exists,
144+
'containers[0].ports': exists,
145+
'containers[0].dockerHost': exists,
146+
'containers[0].dockerContainer': exists,
147+
'containers[0].inspect.State.Running': true
148148
});
149149
done();
150150
});
@@ -155,10 +155,10 @@ describe('PUT /instances/:id/actions/stop', {timeout:1000}, function () {
155155
beforeEach(function (done) {
156156
extend(ctx.expected, {
157157
containers: exists,
158-
'container': exists,
159-
'container.dockerHost': exists,
160-
'container.dockerContainer': exists,
161-
'container.inspect.State.Running': false
158+
'containers[0]': exists,
159+
'containers[0].dockerHost': exists,
160+
'containers[0].dockerContainer': exists,
161+
'containers[0].inspect.State.Running': false
162162
});
163163
ctx.expectAlreadyStopped = true;
164164
ctx.originalStart = Docker.prototype.startContainer;
@@ -232,8 +232,10 @@ describe('PUT /instances/:id/actions/stop', {timeout:1000}, function () {
232232
ctx.instance.stop(expects.error(400, /not have a container/, done));
233233
}
234234
else { // success
235-
ctx.expected['container.inspect.State.Running'] = false;
236-
var assertions = expects.success(200, ctx.expected, startStopAssert);
235+
ctx.expected['containers[0].inspect.State.Running'] = false;
236+
var assertions = ctx.expectAlreadyStopped ?
237+
expects.error(304, startStopAssert) :
238+
expects.success(200, ctx.expected, startStopAssert);
237239
ctx.instance.stop(assertions);
238240
}
239241
function startStopAssert (err) {

0 commit comments

Comments
 (0)