Make start/stop just a installation code

the runState now just tracks if an app is stopped.

src/apptask.js

@@ -489,6 +489,12 @@ function downloadImage(manifest, callback) {
     });
 }
 
+function startApp(app, callback){
+    if (app.runState === apps.RSTATE_STOPPED) return callback();
+
+    docker.startContainer(app.id, callback);
+}
+
 // Ordering is based on the following rationale:
 //   - configure nginx, icon, oauth
 //   - register subdomain.
@@ -520,7 +526,7 @@ function install(app, args, progressCallback, callback) {
         unconfigureReverseProxy.bind(null, app),
         removeCollectdProfile.bind(null, app),
         removeLogrotateConfig.bind(null, app),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function teardownAddons(next) {
             // when restoring, app does not require these addons anymore. remove carefully to preserve the db passwords
@@ -580,7 +586,7 @@ function install(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 80, message: 'Setting up collectd profile' }),
         addCollectdProfile.bind(null, app),
 
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 85, message: 'Waiting for DNS propagation' }),
         exports._waitForDnsPropagation.bind(null, app),
@@ -631,7 +637,7 @@ function create(app, args, progressCallback, callback) {
 
     async.series([
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
 
         // FIXME: re-setup addons only because sendmail addon to re-inject env vars on mailboxName change
@@ -641,8 +647,7 @@ function create(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 60, message: 'Creating container' }),
         createContainer.bind(null, app),
 
-        progressCallback.bind(null, { percent: 80, message: 'Starting app' }),
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
@@ -667,7 +672,7 @@ function changeLocation(app, args, progressCallback, callback) {
     async.series([
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
         unconfigureReverseProxy.bind(null, app),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function (next) {
             let obsoleteDomains = oldConfig.alternateDomains.filter(function (o) {
@@ -691,7 +696,7 @@ function changeLocation(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 60, message: 'Creating container' }),
         createContainer.bind(null, app),
 
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 80, message: 'Waiting for DNS propagation' }),
         exports._waitForDnsPropagation.bind(null, app),
@@ -721,7 +726,7 @@ function migrateDataDir(app, args, progressCallback, callback) {
 
     async.series([
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
 
         progressCallback.bind(null, { percent: 45, message: 'Ensuring app data directory' }),
@@ -743,8 +748,7 @@ function migrateDataDir(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 60, message: 'Creating container' }),
         createContainer.bind(null, app),
 
-        progressCallback.bind(null, { percent: 80, message: 'Starting app' }),
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
@@ -773,7 +777,7 @@ function configure(app, args, progressCallback, callback) {
         unconfigureReverseProxy.bind(null, app),
         removeCollectdProfile.bind(null, app),
         removeLogrotateConfig.bind(null, app),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function (next) {
             let obsoleteDomains = oldConfig.alternateDomains.filter(function (o) {
@@ -821,7 +825,7 @@ function configure(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 70, message: 'Add collectd profile' }),
         addCollectdProfile.bind(null, app),
 
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 80, message: 'Waiting for DNS propagation' }),
         exports._waitForDnsPropagation.bind(null, app),
@@ -883,7 +887,7 @@ function update(app, args, progressCallback, callback) {
         // note: we cleanup first and then backup. this is done so that the app is not running should backup fail
         // we cannot easily 'recover' from backup failures because we have to revert manfest and portBindings
         progressCallback.bind(null, { percent: 35, message: 'Cleaning up old install' }),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function deleteImageIfChanged(done) {
             if (app.manifest.dockerImage === updateConfig.manifest.dockerImage) return done();
@@ -926,7 +930,7 @@ function update(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 80, message: 'Creating container' }),
         createContainer.bind(null, app),
 
-        runApp.bind(null, app, progressCallback),
+        startApp.bind(null, app),
 
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null, updateTime: new Date() })
@@ -945,6 +949,48 @@ function update(app, args, progressCallback, callback) {
     });
 }
 
+function start(app, args, progressCallback, callback) {
+    assert.strictEqual(typeof app, 'object');
+    assert.strictEqual(typeof args, 'object');
+    assert.strictEqual(typeof progressCallback, 'function');
+    assert.strictEqual(typeof callback, 'function');
+
+    async.series([
+        progressCallback.bind(null, { percent: 20, message: 'Starting container' }),
+        docker.startContainer.bind(null, app.id),
+
+        progressCallback.bind(null, { percent: 100, message: 'Done' }),
+        updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
+    ], function seriesDone(error) {
+        if (error) {
+            debugApp(app, 'error starting app: %s', error);
+            return updateApp(app, { installationState: apps.ISTATE_ERROR, error: makeTaskError(error, app, args) }, callback.bind(null, error));
+        }
+        callback(null);
+    });
+}
+
+function stop(app, args, progressCallback, callback) {
+    assert.strictEqual(typeof app, 'object');
+    assert.strictEqual(typeof args, 'object');
+    assert.strictEqual(typeof progressCallback, 'function');
+    assert.strictEqual(typeof callback, 'function');
+
+    async.series([
+        progressCallback.bind(null, { percent: 20, message: 'Stopping container' }),
+        docker.stopContainers.bind(null, app.id),
+
+        progressCallback.bind(null, { percent: 100, message: 'Done' }),
+        updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
+    ], function seriesDone(error) {
+        if (error) {
+            debugApp(app, 'error starting app: %s', error);
+            return updateApp(app, { installationState: apps.ISTATE_ERROR, error: makeTaskError(error, app, args) }, callback.bind(null, error));
+        }
+        callback(null);
+    });
+}
+
 function uninstall(app, args, progressCallback, callback) {
     assert.strictEqual(typeof app, 'object');
     assert.strictEqual(typeof args, 'object');
@@ -959,7 +1005,7 @@ function uninstall(app, args, progressCallback, callback) {
         removeLogrotateConfig.bind(null, app),
 
         progressCallback.bind(null, { percent: 10, message: 'Stopping app' }),
-        stopApp.bind(null, app, progressCallback),
+        docker.stopContainers.bind(null, app.id),
 
         progressCallback.bind(null, { percent: 20, message: 'Deleting container' }),
         deleteContainers.bind(null, app, {}),
@@ -996,34 +1042,6 @@ function uninstall(app, args, progressCallback, callback) {
     });
 }
 
-function runApp(app, progressCallback, callback) {
-    assert.strictEqual(typeof app, 'object');
-    assert.strictEqual(typeof progressCallback, 'function');
-    assert.strictEqual(typeof callback, 'function');
-
-    progressCallback({ message: 'Starting app' });
-
-    docker.startContainer(app.containerId, function (error) {
-        if (error) return callback(new BoxError(BoxError.DOCKER_ERROR, `Error starting container: ${error.message}`));
-
-        updateApp(app, { runState: apps.RSTATE_RUNNING }, callback);
-    });
-}
-
-function stopApp(app, progressCallback, callback) {
-    assert.strictEqual(typeof app, 'object');
-    assert.strictEqual(typeof progressCallback, 'function');
-    assert.strictEqual(typeof callback, 'function');
-
-    progressCallback({ message: 'Stopping app' });
-
-    docker.stopContainers(app.id, function (error) {
-        if (error) return callback(new BoxError(BoxError.DOCKER_ERROR, `Error starting container: ${error.message}`));
-
-        updateApp(app, { runState: apps.RSTATE_STOPPED, health: null }, callback);
-    });
-}
-
 function run(appId, args, progressCallback, callback) {
     assert.strictEqual(typeof appId, 'string');
     assert.strictEqual(typeof args, 'object');
@@ -1065,13 +1083,10 @@ function run(appId, args, progressCallback, callback) {
             return update(app, args, progressCallback, callback);
         case apps.ISTATE_PENDING_BACKUP:
             return backup(app, args, progressCallback, callback);
-        case apps.ISTATE_INSTALLED:
-            switch (app.runState) {
-            case apps.RSTATE_PENDING_STOP: return stopApp(app, progressCallback, callback);
-            case apps.RSTATE_PENDING_START: return runApp(app, progressCallback, callback);
-            default: return callback(new Error('Unknown run command in apptask:' + app.runState));
-            }
-
+        case apps.ISTATE_PENDING_START:
+            return start(app, args, progressCallback, callback);
+        case apps.ISTATE_PENDING_STOP:
+            return stop(app, args, progressCallback, callback);
         default:
             debugApp(app, 'apptask launched with invalid command');
             return callback(new Error('Unknown install command in apptask:' + app.installationState));