diff --git a/src/appdb.js b/src/appdb.js index 89a724d9d..7ebcc683a 100644 --- a/src/appdb.js +++ b/src/appdb.js @@ -454,16 +454,18 @@ function setHealth(appId, health, healthTime, callback) { updateWithConstraints(appId, values, '', callback); } -function setTask(appId, values, requiredState, callback) { +function setTask(appId, values, options, callback) { assert.strictEqual(typeof appId, 'string'); assert.strictEqual(typeof values, 'object'); - assert(requiredState === null || typeof requiredState === 'string'); + assert.strictEqual(typeof options, 'object'); assert.strictEqual(typeof callback, 'function'); - if (requiredState === null) { + if (!options.requireNullTaskId) return updateWithConstraints(appId, values, '', callback); + + if (options.requiredState === null) { updateWithConstraints(appId, values, 'AND taskId IS NULL', callback); } else { - updateWithConstraints(appId, values, `AND taskId IS NULL AND installationState = "${requiredState}"`, callback); + updateWithConstraints(appId, values, `AND taskId IS NULL AND installationState = "${options.requiredState}"`, callback); } } diff --git a/src/apps.js b/src/apps.js index ab65bc4fe..ccdc40189 100644 --- a/src/apps.js +++ b/src/apps.js @@ -54,7 +54,7 @@ exports = module.exports = { restoreInstalledApps: restoreInstalledApps, configureInstalledApps: configureInstalledApps, - resumeTasks: resumeTasks, + schedulePendingTasks: schedulePendingTasks, getDataDir: getDataDir, getIconPath: getIconPath, @@ -655,16 +655,18 @@ function addTask(appId, installationState, task, callback) { const { args, values } = task; // by default, a task can only run on installed state. if it's null, it can be run on any state const requiredState = 'requiredState' in task ? task.requiredState : exports.ISTATE_INSTALLED; + const scheduleNow = 'scheduleNow' in task ? task.scheduleNow : true; + const requireNullTaskId = 'requireNullTaskId' in task ? task.requireNullTaskId : true; tasks.add(tasks.TASK_APP, [ appId, args ], function (error, taskId) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - appdb.setTask(appId, _.extend({ installationState, taskId, error: null }, values), requiredState, function (error) { + appdb.setTask(appId, _.extend({ installationState, taskId, error: null }, values), { requiredState, requireNullTaskId }, function (error) { if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new AppsError(AppsError.ALREADY_EXISTS, error.message)); if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE, 'Another task is scheduled for this app')); // could be because app went away OR a taskId exists if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - scheduleTask(appId, installationState, taskId, NOOP_CALLBACK); + if (scheduleNow) scheduleTask(appId, installationState, taskId, NOOP_CALLBACK); callback(null, { taskId }); }); @@ -1843,21 +1845,25 @@ function restoreInstalledApps(callback) { getAll(function (error, apps) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); apps = apps.filter(app => app.installationState !== exports.ISTATE_ERROR); // remove errored apps. let them be 'repaired' by hand + apps = apps.filter(app => app.installationState !== exports.ISTATE_PENDING_RESTORE); // safeguard against tasks being created non-stop if we crash on startup - async.map(apps, function (app, iteratorDone) { + async.eachSeries(apps, function (app, iteratorDone) { backups.getByAppIdPaged(1, 1, app.id, function (error, results) { const restoreConfig = !error && results.length ? { backupId: results[0].id, backupFormat: results[0].format, oldManifest: app.manifest } : null; + const task = { + args: { restoreConfig, overwriteDns: true }, + values: {}, + scheduleNow: false, // task will be scheduled by autoRestartTasks when platform is ready + requireNullTaskId: false // ignore existing stale taskId + }; - debug(`marking ${app.fqdn} for restore using restore config ${JSON.stringify(restoreConfig)}`); + debug(`restoreInstalledApps: marking ${app.fqdn} for restore using restore config ${JSON.stringify(restoreConfig)}`); - appdb.update(app.id, { taskId: null, error: null }, function (error) { // clear any stale taskId - if (error) debug(`Error marking ${app.fqdn} for restore: ${JSON.stringify(error)}`); + addTask(app.id, exports.ISTATE_PENDING_RESTORE, task, function (error, result) { + if (error) debug(`restoreInstalledApps: error marking ${app.fqdn} for restore: ${JSON.stringify(error)}`); + else debug(`restoreInstalledApps: marked ${app.id} for restore with taskId ${result.taskId}`); - const task = { - args: { restoreConfig, overwriteDns: true }, - values: {} - }; - addTask(app.id, exports.ISTATE_PENDING_RESTORE, task, () => iteratorDone()); // always succeed + iteratorDone(); // ignore error }); }); }, callback); @@ -1870,31 +1876,34 @@ function configureInstalledApps(callback) { getAll(function (error, apps) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); apps = apps.filter(app => app.installationState !== exports.ISTATE_ERROR); // remove errored apps. let them be 'repaired' by hand + apps = apps.filter(app => app.installationState !== exports.ISTATE_PENDING_CONFIGURE); // safeguard against tasks being created non-stop if we crash on startup - async.map(apps, function (app, iteratorDone) { - debug(`marking ${app.fqdn} for reconfigure`); + async.eachSeries(apps, function (app, iteratorDone) { + debug(`configureInstalledApps: marking ${app.fqdn} for reconfigure`); - appdb.update(app.id, { taskId: null, error: null }, function (error) { // clear any stale taskId - if (error) debug(`Error marking ${app.fqdn} for reconfigure: ${JSON.stringify(error)}`); + const oldConfig = _.pick(app, 'location', 'domain', 'fqdn', 'alternateDomains', 'portBindings'); + const task = { + args: { oldConfig }, + values: {}, + scheduleNow: false, // task will be scheduled by autoRestartTasks when platform is ready + requireNullTaskId: false // ignore existing stale taskId + }; - const oldConfig = _.pick(app, 'location', 'domain', 'fqdn', 'alternateDomains', 'portBindings'); - const task = { - args: { oldConfig }, - values: {} - }; - addTask(app.id, exports.ISTATE_PENDING_CONFIGURE, task, () => iteratorDone()); // always succeed + addTask(app.id, exports.ISTATE_PENDING_CONFIGURE, task, function (error, result) { + if (error) debug(`configureInstalledApps: error marking ${app.fqdn} for configure: ${JSON.stringify(error)}`); + else debug(`configureInstalledApps: marked ${app.id} for re-configure with taskId ${result.taskId}`); + + iteratorDone(); // ignore error }); }, callback); }); } -// resume app tasks when platform is ready or after a crash -function resumeTasks(callback) { +// auto-restart app tasks after a crash +function schedulePendingTasks(callback) { assert.strictEqual(typeof callback, 'function'); - debug('resuming tasks'); - - appTaskManager.initializeSync(); + debug('schedulePendingTasks: scheduling app tasks'); getAll(function (error, result) { if (error) return callback(error); @@ -1902,7 +1911,7 @@ function resumeTasks(callback) { result.forEach(function (app) { if (!app.taskId) return; // if not in any pending state, do nothing - debug(`resumeTask: schedule task for ${app.fqdn} ${app.id}: state=${app.installationState},taskId=${app.taskId}`); + debug(`schedulePendingTasks: schedule task for ${app.fqdn} ${app.id}: state=${app.installationState},taskId=${app.taskId}`); scheduleTask(app.id, app.installationState, app.taskId, NOOP_CALLBACK); }); diff --git a/src/apptaskmanager.js b/src/apptaskmanager.js index 587e292bb..ed2d507cd 100644 --- a/src/apptaskmanager.js +++ b/src/apptaskmanager.js @@ -1,7 +1,6 @@ 'use strict'; exports = module.exports = { - initializeSync: initializeSync, scheduleTask: scheduleTask }; @@ -16,6 +15,7 @@ let assert = require('assert'), let gActiveTasks = { }; // indexed by app id let gPendingTasks = [ ]; +let gInitialized = false; const TASK_CONCURRENCY = 3; const NOOP_CALLBACK = function (error) { if (error) debug(error); }; @@ -28,12 +28,19 @@ function waitText(lockOperation) { return ''; // cannot happen } +function initializeSync() { + gInitialized = true; + locker.on('unlocked', startNextTask); +} + // callback is called when task is finished function scheduleTask(appId, taskId, callback) { assert.strictEqual(typeof appId, 'string'); assert.strictEqual(typeof taskId, 'string'); assert.strictEqual(typeof callback, 'function'); + if (!gInitialized) initializeSync(); + if (appId in gActiveTasks) { return callback(new Error(`Task for %s is already active: ${appId}`)); } @@ -77,6 +84,3 @@ function startNextTask() { scheduleTask(t.appId, t.taskId, t.callback); } -function initializeSync() { - locker.on('unlocked', startNextTask); -} diff --git a/src/platform.js b/src/platform.js index 1b9293fdf..84876e5e1 100644 --- a/src/platform.js +++ b/src/platform.js @@ -81,7 +81,8 @@ function stop(callback) { function onPlatformReady() { debug('onPlatformReady: platform is ready'); exports._isReady = true; - apps.resumeTasks(NOOP_CALLBACK); + + apps.schedulePendingTasks(NOOP_CALLBACK); applyPlatformConfig(NOOP_CALLBACK); pruneInfraImages(NOOP_CALLBACK); diff --git a/src/routes/test/backups-test.js b/src/routes/test/backups-test.js index 3dd9fee42..8e34aefa0 100644 --- a/src/routes/test/backups-test.js +++ b/src/routes/test/backups-test.js @@ -58,7 +58,7 @@ function setup(done) { function addApp(callback) { var manifest = { version: '0.0.1', manifestVersion: 1, dockerImage: 'foo', healthCheckPath: '/', httpPort: 3, title: 'ok', addons: { } }; - appdb.add('appid', 'appStoreId', manifest, 'location', DOMAIN_0.domain, [ ] /* portBindings */, { }, callback); + appdb.add('appid', 'appStoreId', manifest, 'location', DOMAIN_0.domain, [ ] /* portBindings */, { installationState: 'installed', runState: 'running' }, callback); }, function createSettings(callback) { diff --git a/src/routes/test/oauth2-test.js b/src/routes/test/oauth2-test.js index 44451d85b..2424d37c2 100644 --- a/src/routes/test/oauth2-test.js +++ b/src/routes/test/oauth2-test.js @@ -66,7 +66,9 @@ describe('OAuth2', function () { domain: DOMAIN_0.domain, portBindings: {}, accessRestriction: null, - memoryLimit: 0 + memoryLimit: 0, + installationState: 'pending_install', + runState: 'running' }; var APP_1 = { @@ -77,7 +79,9 @@ describe('OAuth2', function () { domain: DOMAIN_0.domain, portBindings: {}, accessRestriction: { users: [ 'foobar' ] }, - memoryLimit: 0 + memoryLimit: 0, + installationState: 'pending_install', + runState: 'running' }; var APP_2 = { @@ -88,7 +92,9 @@ describe('OAuth2', function () { domain: DOMAIN_0.domain, portBindings: {}, accessRestriction: { users: [ USER_0.id ] }, - memoryLimit: 0 + memoryLimit: 0, + installationState: 'pending_install', + runState: 'running' }; var APP_3 = { @@ -99,7 +105,9 @@ describe('OAuth2', function () { domain: DOMAIN_0.domain, portBindings: {}, accessRestriction: { groups: [ 'someothergroup', 'admin', 'anothergroup' ] }, - memoryLimit: 0 + memoryLimit: 0, + installationState: 'pending_install', + runState: 'running' }; // unknown app diff --git a/src/test/apps-test.js b/src/test/apps-test.js index c89916415..392acf0da 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -118,7 +118,9 @@ describe('Apps', function () { env: { 'CUSTOM_KEY': 'CUSTOM_VALUE' }, - dataDir: '' + dataDir: '', + installationState: 'installed', + runState: 'running' }; var APP_1 = { @@ -135,7 +137,9 @@ describe('Apps', function () { accessRestriction: { users: [ 'someuser' ], groups: [ GROUP_0.id ] }, memoryLimit: 0, env: {}, - dataDir: '' + dataDir: '', + installationState: 'installed', + runState: 'running' }; var APP_2 = { @@ -154,7 +158,9 @@ describe('Apps', function () { robotsTxt: null, sso: false, env: {}, - dataDir: '' + dataDir: '', + installationState: 'installed', + runState: 'running' }; before(function (done) { @@ -396,7 +402,7 @@ describe('Apps', function () { ], done); }); - it('can mark apps for reconfigure', function (done) { + it('can mark apps for restore', function (done) { apps.restoreInstalledApps(function (error) { expect(error).to.be(null); diff --git a/src/test/apptask-test.js b/src/test/apptask-test.js index 899f5d70a..1fcf191df 100644 --- a/src/test/apptask-test.js +++ b/src/test/apptask-test.js @@ -83,7 +83,7 @@ var APP = { id: 'appid', appStoreId: 'appStoreId', installationState: apps.ISTATE_PENDING_INSTALL, - runState: null, + runState: 'running', location: 'applocation', domain: DOMAIN_0.domain, fqdn: DOMAIN_0.domain + '.' + 'applocation', diff --git a/src/test/updatechecker-test.js b/src/test/updatechecker-test.js index a4f1b7831..fc75959fe 100644 --- a/src/test/updatechecker-test.js +++ b/src/test/updatechecker-test.js @@ -218,7 +218,7 @@ describe('updatechecker - app - manual (email)', function () { appStoreId: 'io.cloudron.app', installationState: apps.ISTATE_PENDING_INSTALL, error: null, - runState: null, + runState: 'running', location: 'some-location-0', domain: DOMAIN_0.domain, manifest: { @@ -325,7 +325,7 @@ describe('updatechecker - app - automatic (no email)', function () { appStoreId: 'io.cloudron.app', installationState: apps.ISTATE_PENDING_INSTALL, error: null, - runState: null, + runState: 'running', location: 'some-location-0', domain: DOMAIN_0.domain, manifest: { @@ -388,7 +388,7 @@ describe('updatechecker - app - automatic free (email)', function () { appStoreId: 'io.cloudron.app', installationState: apps.ISTATE_PENDING_INSTALL, error: null, - runState: null, + runState: 'running', location: 'some-location-0', domain: DOMAIN_0.domain, manifest: {