Merge pull request #383 from CodeNow/image-pull

anandkumarpatel · anandkumarpatel · commit cb806d884092 · 2014-12-16T14:35:40.000-08:00
Image pull
diff --git a/lib/models/apis/docker.js b/lib/models/apis/docker.js
@@ -264,22 +264,45 @@ Docker.prototype.getLogs = function (containerId, tail, cb) {
   }
 };
 
-
-
-Docker.prototype.createAndInspectContainer = function (version, opts, cb) {
-  debug('createAndInspectContainer', formatArgs(arguments));
-  var self = this;
-  if (typeof opts === 'function') {
+/**
+ * creates a user container
+ * @param version: version object which containes build
+ * @param opts:    opts to pass to docker
+ * @param cb: Callback
+ */
+Docker.prototype.createUserContainer = function (version, opts, cb) {
+  debug('createUserContainer', formatArgs(arguments));
+  if (!version.build || !version.build.dockerTag) {
+    return cb(Boom.badRequest('Cannot create a container for an unbuilt version', {
+      debug: { versionId: version._id.toString() }
+    }));
+  }
+  if (isFunction(opts)) {
     cb = opts;
     opts = {};
   }
-  // Create opts
-  opts.create = opts.create || {};
-  extend(opts.create, {
+  extend(opts, {
     Image: version.build.dockerTag
   });
-  // Start opts
-  opts.start = opts.start || {};
+
+  this.createContainer(opts, cb);
+};
+
+/**
+ * start a user container
+ * @param container: container object to start
+ * @param opts:      opts to pass to docker
+ * @param cb: Callback
+ */
+Docker.prototype.startUserContainer = function (container, opts, cb) {
+  debug('startUserContainer', formatArgs(arguments));
+  if (!container) {
+    return cb(Boom.badRequest('Container must be provided to start'));
+  }
+  if (isFunction(opts)) {
+    cb = opts;
+    opts = {};
+  }
 
   // order matters here , our custom DNS should come first
   var dns = [];
@@ -288,40 +311,31 @@ Docker.prototype.createAndInspectContainer = function (version, opts, cb) {
   }
   dns.push(process.env.DNS_DEFAULT_IPADDRESS);
 
-  extend(opts.start, {
+  extend(opts, {
     PublishAllPorts: true,
     Dns: dns
   });
 
-  async.waterfall([
-    createContainer,
-    startContainer
-  ], cb);
-  function createContainer (cb) {
-    self.createContainer(opts.create, cb);
-  }
-  function startContainer (container, cb) {
-    self.startContainer(container, opts.start, function (err) {
-      cb(err, container);
-    });
-  }
+  this.startContainer(container, opts, cb);
 };
 
-Docker.prototype.createContainerForVersion = function (version, opts, cb) {
-  debug('createContainerForVersion', formatArgs(arguments));
-  if (typeof opts === 'function') {
-    cb = opts;
-    opts = {};
-  }
-  if (!version.build || !version.build.dockerImage) {
-    return cb(Boom.badRequest('Cannot create a container for an unbuilt version', {
-      debug: { versionId: version._id.toString() }
-    }));
+/**
+ * inspect a user container
+ * @param container: container object to inspect
+ * @param cb: Callback
+ */
+Docker.prototype.inspectUserContainer = function (container, cb) {
+  debug('inspectUserContainer', formatArgs(arguments));
+  if (!container) {
+    return cb(Boom.badRequest('Container must be provided to start'));
   }
-  this.createAndInspectContainer(version, opts, cb);
+  var self = this;
+  self.inspectContainer(container, function(err, inspect) {
+    if (err) { return cb(err); }
+    inspect.dockerHost = self.dockerHost;
+    cb(null, inspect);
+  });
 };
-
-
 /**
  * CONTAINER METHODS - START
  */
@@ -334,7 +348,7 @@ Docker.prototype.createContainerForVersion = function (version, opts, cb) {
 Docker.prototype.createContainer = function (opts, cb) {
   debug('createContainer', formatArgs(arguments));
   var self = this;
-  if (typeof opts === 'function') {
+  if (isFunction(opts)) {
     cb = opts;
     opts = {};
   }
@@ -370,7 +384,7 @@ Docker.prototype.startContainer = function (container, opts, cb) {
   debug('startContainer', formatArgs(arguments));
   var self = this;
   var containerId = container.dockerContainer || container.Id;
-  if (typeof opts === 'function') {
+  if (isFunction(opts)) {
     cb = opts;
     opts = {};
   }
@@ -395,6 +409,17 @@ Docker.prototype.restartContainer = function (container, cb) {
       self.handleErr(cb, 'Restart container failed', { containerId: containerId }));
 };
 
+/**
+ * pulls image and returns stream
+ * @param contextVersion - format 'myrepo/myname:tag'
+ * @param cb: Callback
+ */
+Docker.prototype.pullImage = function (version, cb) {
+  debug('pullImage', formatArgs(arguments));
+  var self = this;
+  self.docker.pull(version.build.dockerTag, cb);
+};
+
 /**
  * attempts to stop a running container.
  * if not stopped in passed in time, the process is kill 9'd
diff --git a/lib/models/apis/runnable.js b/lib/models/apis/runnable.js
@@ -10,6 +10,8 @@ var RunnableUser = require('runnable');
 var Base = require('runnable/lib/models/base');
 var debug = require('debug')('runnable-api:runnable:model');
 
+var isFunction = require('101/is-function');
+
 Base.prototype.parse = function (attrs) {
   if (attrs.toJSON) {
     attrs = attrs.toJSON();
@@ -61,25 +63,20 @@ util.inherits(Runnable, RunnableUser);
  * @param instances
  * @param cb
  */
-Runnable.prototype.redeployInstance = function (instance, buildId, cb) {
+Runnable.prototype.redeployInstance = function (instance, opts, cb) {
   debug('redeployInstance', formatArgs(arguments));
-  if (typeof buildId === 'function') {
-    // (instance, cb)
-    cb = buildId;
-    buildId = null;
+  if (isFunction(opts)) {
+    cb = opts;
+    opts = {};
   }
-  var opts = (buildId) ?
-    { json: { build: buildId.toString() } } :
-    {};
-
   var instanceModel = this.newInstance(instance.shortHash);
   var retries = 0;
   var maxRetries = 15;
   redeploy();
   function redeploy () {
     instanceModel.redeploy(opts, function (err) {
       if (err) {
-        if (err.output.statusCode === 409 && buildId) {
+        if (err.output.statusCode === 409 && opts.json.build) {
           // only attempt retries if buildId is provided
           setTimeout(function () {
             attemptRetry(err);
diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js
@@ -243,7 +243,9 @@ InstanceSchema.methods.modifyUnsetContainer = function (cb) {
 InstanceSchema.methods.modifyContainerCreateErr = function (err, cb) {
   Instance.findByIdAndUpdate(this._id, {
     $set: {
-      'container.error': pick(err, ['message', 'stack', 'data'])
+      container: {
+        error: pick(err, ['message', 'stack', 'data', 'imageIsPulling'])
+      }
     }
   }, cb);
 };
diff --git a/lib/models/mongo/schemas/instance.js b/lib/models/mongo/schemas/instance.js
@@ -130,7 +130,8 @@ var InstanceSchema = module.exports = new Schema({
         type: {
           message: String,
           stack: String,
-          data: Schema.Types.Mixed
+          data: Schema.Types.Mixed,
+          imageIsPulling: Boolean
         }
       }
     }
diff --git a/lib/routes/builds.js b/lib/routes/builds.js
@@ -282,7 +282,7 @@ app.post('/builds/:id/actions/build',
         next();
       },
       runnable.create({}, 'sessionUser'),
-      runnable.model.redeployInstance('instance', 'build._id'))),
+      runnable.model.redeployInstance('instance', { json: { build: 'build._id.toString()' } }))),
   // noop is required!! to prevent res.send(404) after the response has already been sent.
   noop
 );
diff --git a/lib/routes/instances/index.js b/lib/routes/instances/index.js
@@ -113,22 +113,51 @@ var bodyValidations = flow.series(
 var createSaveAndNetworkContainer = flow.series(
   // host LOCK must be acquired PRIOR to calling this (not necessary for post instances)!
   findBuildContextVersion,
-  mavis.create(),
-  mavis.model.findDock('container_run', 'contextVersion.dockerHost'),
-  mw.req().set('dockerHost', 'mavisResult'),
+  mw.req('body.forceDock').require()
+    .then(
+      mw.req().set('dockerHost', 'body.forceDock'))
+    .else(
+      mavis.create(),
+      mavis.model.findDock('container_run', 'contextVersion.dockerHost'),
+      mw.req().set('dockerHost', 'mavisResult')),
   flow.try(
     docker.create('dockerHost'),
-    docker.model.createContainerForVersion('contextVersion', {
-      create: { Env: 'instance.env' }
-    }),
+    docker.model.createUserContainer('contextVersion', { Env: 'instance.env' }),
     mw.req().set('containerInfo', 'dockerResult'))
   .catch(
     mw.req().setToErr('containerCreateErr'),
-    instances.model.modifyContainerCreateErr('containerCreateErr')),
-  // sauron attach container to ip
-  mw.req('containerCreateErr').require()
-    .else( // container create was successful
-      instances.model.inspectAndUpdate('containerInfo', 'dockerHost'),
+    mw.req('containerCreateErr.output.statusCode').validate(validations.equals(404))
+      .then(
+        mw.req().set('containerCreateErr.imageIsPulling', true)),
+    instances.model.modifyContainerCreateErr('containerCreateErr'),
+    mw.req('containerCreateErr.imageIsPulling').validate(validations.equals(true))
+      .then(
+        docker.model.pullImage('contextVersion'),
+        function(req, res, next) {
+          // emitter.emit('dockerResult') need to emit pull event here
+          req.dockerResult.on('data', function () {
+            // TODO: pipe to pull-stream
+          });
+          // on pull finish, redeploy instance
+          req.dockerResult.on('end', function () {
+            flow.series(
+              runnable.create({}, 'sessionUser'),
+              runnable.model.redeployInstance('instance', {
+                forceDock: 'dockerHost',
+                json: { build: 'build._id.toString()' }
+              }))(req, res, function (err) {
+                error.logIfErr(err, req);
+              });
+          });
+          // next immediately, background pull and redeploy
+          next();
+        })),
+  mw.req('containerInfo').require()
+    .then( // container create was successful
+      docker.model.startUserContainer('containerInfo'),
+      docker.model.inspectUserContainer('containerInfo'),
+      mw.req().set('containerInspect', 'dockerResult'),
+      instances.model.modifySetContainer('containerInspect', 'dockerHost'),
       sauron.create('dockerHost'),
       sauron.model.attachHostToContainer(
         'instance.network.networkIp',
@@ -260,7 +289,7 @@ app.post('/instances/',
       .then(
         // deploy build if it is built
         runnable.create({}, 'sessionUser'),
-        runnable.model.redeployInstance('instance', 'build._id'),
+        runnable.model.redeployInstance('instance', { json: { build: 'build._id.toString()' } }),
         mw.res.send(201, 'runnableResult'),
         function () {}))
   .catch( // dealloc networkIp/hostIp
diff --git a/test/instances/post/201.js b/test/instances/post/201.js
@@ -58,6 +58,21 @@ describe('201 POST /instances', {timeout:500}, function () {
       cb(createErr);
     }
   };
+  var forceImageNotFoundOnCreateErrOnce = function(oldFn) {
+    return function () {
+      var cb = last(arguments);
+      var createErr = new Error("image not found");
+      extend(createErr, {
+        statusCode : 404,
+        reason     : "no such container",
+        json       : "no such container\n"
+      });
+      if (isFunction(cb)) {
+        cb(createErr);
+      }
+      Dockerode.prototype.createContainer = oldFn;
+    };
+  };
   function initExpected (done) {
     ctx.expected = {
       _id: exists,
@@ -223,6 +238,21 @@ describe('201 POST /instances', {timeout:500}, function () {
         });
         createInstanceTests(ctx);
       });
+      describe('Container create error (Image not on dock)', function() {
+        beforeEach(function (done) {
+          extend(ctx.expected, {
+            containers: exists,
+            'containers[0].error.message': exists,
+            'containers[0].error.stack': exists,
+            'containers[0].error.imageIsPulling': true
+          });
+          Dockerode.prototype.createContainer = forceImageNotFoundOnCreateErrOnce(Dockerode.prototype.createContainer);
+          done();
+        });
+
+        createInstanceTests(ctx);
+      });
+
     });
   });
   describe('for Organization by member', function () {

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,8 @@ var InstanceSchema = module.exports = new Schema({`
`130`	`130`	`type: {`
`131`	`131`	`message: String,`
`132`	`132`	`stack: String,`
`133`		`- data: Schema.Types.Mixed`
	`133`	`+ data: Schema.Types.Mixed,`
	`134`	`+ imageIsPulling: Boolean`
`134`	`135`	`}`
`135`	`136`	`}`
`136`	`137`	`}`