diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index d0261fa0c..1ae4f5253 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -4143,9 +4143,9 @@ "optional": true }, "safetydance": { - "version": "0.1.1", - "from": "safetydance@>=0.1.1 <0.2.0", - "resolved": "https://registry.npmjs.org/safetydance/-/safetydance-0.1.1.tgz" + "version": "0.2.0", + "from": "safetydance@0.2.0", + "resolved": "https://registry.npmjs.org/safetydance/-/safetydance-0.2.0.tgz" }, "sass-graph": { "version": "2.1.2", diff --git a/package.json b/package.json index ddce4f71b..8a3edec1e 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "password-generator": "^2.0.2", "progress-stream": "^2.0.0", "proxy-middleware": "^0.13.0", - "safetydance": "^0.1.1", + "safetydance": "^0.2.0", "semver": "^4.3.6", "showdown": "^1.6.0", "split": "^1.0.0", diff --git a/src/backupdb.js b/src/backupdb.js index 801be3e01..958a00937 100644 --- a/src/backupdb.js +++ b/src/backupdb.js @@ -114,8 +114,19 @@ function del(id, callback) { assert.strictEqual(typeof id, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('DELETE FROM backups WHERE id=?', [ id ], function (error) { - if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); - callback(null); + get(id, function (error, result) { + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(); + if (error) return callback(error); + + var whereClause = [ 'id=?' ], whereArgs = [ result.id ]; + result.dependsOn.forEach(function (id) { + whereClause.push('id=?'); + whereArgs.push(id); + }); + + database.query('DELETE FROM backups WHERE ' + whereClause.join(' OR '), whereArgs, function (error) { + if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); + callback(null); + }); }); } diff --git a/src/backups.js b/src/backups.js index 0deb7fd84..e82710ce6 100644 --- a/src/backups.js +++ b/src/backups.js @@ -18,7 +18,7 @@ exports = module.exports = { backupBoxAndApps: backupBoxAndApps, - removeBackup: removeBackup + cleanup: cleanup }; var addons = require('./addons.js'), @@ -421,25 +421,47 @@ function restoreApp(app, addonsToRestore, backupId, callback) { }); } -function removeBackup(backupId, appBackupIds, callback) { - assert.strictEqual(typeof backupId, 'string'); - assert(util.isArray(appBackupIds)); - assert.strictEqual(typeof callback, 'function'); +function cleanup(callback) { + assert(!callback || typeof callback === 'function'); // callback is null when called from cronjob - debug('removeBackup: %s', backupId); + callback = callback || NOOP_CALLBACK; settings.getBackupConfig(function (error, backupConfig) { - if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); + if (error) return callback(error); - api(backupConfig.provider).removeBackup(backupConfig, backupId, appBackupIds, function (error) { + if (backupConfig.retentionSecs < 0) { + debug('cleanup: keeping all backups'); + return callback(); + } + + getPaged(1, 1000, function (error, result) { if (error) return callback(error); - backupdb.del(backupId, function (error) { - if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); + var now = new Date(); - debug('removeBackup: %s done', backupId); + async.eachSeries(result, function iterator(backup, iteratorDone) { + if ((now - backup.creationTime) < (backupConfig.retentionSecs * 1000)) return iteratorDone(); - callback(null); + debug('cleanup: removing %j', backup.id); + + var backupIds = [].concat(backup.id, backup.dependsOn); + + api(backupConfig.provider).removeBackups(backupConfig, backupIds, function (error) { + if (error) { + debug('cleanup: error removing backup %j : %s', backup, error.message); + iteratorDone(); + } + + backupdb.del(backup.id, function (error) { + if (error) debug('cleanup: error removing from database', error); + else debug('cleanup: removed %j', backupIds); + + iteratorDone(); + }); + }); + }, function () { + debug('cleanup: done cleaning backups'); + callback(); }); }); }); diff --git a/src/cron.js b/src/cron.js index 4fe8297fb..9478f7368 100644 --- a/src/cron.js +++ b/src/cron.js @@ -135,8 +135,8 @@ function recreateJobs(tz) { if (gCleanupBackupsJob) gCleanupBackupsJob.stop(); gCleanupBackupsJob = new CronJob({ - cronTime: '00 */30 * * * *', // every 30 minutes - onTick: janitor.cleanupBackups, + cronTime: '00 00 */4 * * *', // every 4 hours + onTick: backups.cleanup, start: true, timeZone: tz }); diff --git a/src/janitor.js b/src/janitor.js index a0c596eca..ff9e51758 100644 --- a/src/janitor.js +++ b/src/janitor.js @@ -3,16 +3,13 @@ var assert = require('assert'), async = require('async'), authcodedb = require('./authcodedb.js'), - backups = require('./backups.js'), debug = require('debug')('box:src/janitor'), docker = require('./docker.js').connection, - settings = require('./settings.js'), tokendb = require('./tokendb.js'); exports = module.exports = { cleanupTokens: cleanupTokens, - cleanupDockerVolumes: cleanupDockerVolumes, - cleanupBackups: cleanupBackups + cleanupDockerVolumes: cleanupDockerVolumes }; var NOOP_CALLBACK = function () { }; @@ -104,38 +101,3 @@ function cleanupDockerVolumes(callback) { }, callback); }); } - -function cleanupBackups(callback) { - assert(!callback || typeof callback === 'function'); // callback is null when called from cronjob - - callback = callback || NOOP_CALLBACK; - - debug('Cleaning backups'); - - settings.getBackupConfig(function (error, backupConfig) { - if (error) return callback(error); - - if (backupConfig.retentionSecs < 0) return callback(); - - backups.getPaged(1, 1000, function (error, result) { - if (error) return callback(error); - - var now = new Date(); - var toCleanup = result.filter(function (a) { - return (now - a.creationTime) > (backupConfig.retentionSecs * 1000); - }); - - debug('cleanupBackups: about to clean: %j', toCleanup); - - async.each(toCleanup, function (backup, callback) { - backups.removeBackup(backup.id, backup.dependsOn, function (error) { - if (error) console.error(error); - - debug('cleanupBackups: %s, %s done', backup.id, backup.dependsOn.join(', ')); - - callback(); - }); - }, callback); - }); - }); -} diff --git a/src/storage/caas.js b/src/storage/caas.js index 1565848a4..155c5e667 100644 --- a/src/storage/caas.js +++ b/src/storage/caas.js @@ -4,7 +4,7 @@ exports = module.exports = { backup: backup, restore: restore, copyBackup: copyBackup, - removeBackup: removeBackup, + removeBackups: removeBackups, backupDone: backupDone, @@ -159,10 +159,9 @@ function copyBackup(apiConfig, oldBackupId, newBackupId, callback) { }); } -function removeBackup(apiConfig, backupId, appBackupIds, callback) { +function removeBackups(apiConfig, backupIds, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert.strictEqual(typeof backupId, 'string'); - assert(Array.isArray(appBackupIds)); + assert(Array.isArray(backupIds)); assert.strictEqual(typeof callback, 'function'); getBackupCredentials(apiConfig, function (error, credentials) { @@ -170,11 +169,17 @@ function removeBackup(apiConfig, backupId, appBackupIds, callback) { var params = { Bucket: apiConfig.bucket, - Key: getBackupFilePath(apiConfig, backupId) + Delete: { + Objects: [ ] // { Key } + } }; + backupIds.forEach(function (backupId) { + params.Delete.Objects.push({ Key: getBackupFilePath(apiConfig, backupId) }); + }); + var s3 = new AWS.S3(credentials); - s3.deleteObject(params, function (error) { + s3.deleteObjects(params, function (error) { if (error) console.error('Unable to remove %s. Not fatal.', params.Key, error); callback(null); }); diff --git a/src/storage/filesystem.js b/src/storage/filesystem.js index ddb075075..297d299a0 100644 --- a/src/storage/filesystem.js +++ b/src/storage/filesystem.js @@ -4,7 +4,7 @@ exports = module.exports = { backup: backup, restore: restore, copyBackup: copyBackup, - removeBackup: removeBackup, + removeBackups: removeBackups, backupDone: backupDone, @@ -136,19 +136,23 @@ function copyBackup(apiConfig, oldBackupId, newBackupId, callback) { }); } -function removeBackup(apiConfig, backupId, appBackupIds, callback) { +function removeBackups(apiConfig, backupIds, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert.strictEqual(typeof backupId, 'string'); - assert(Array.isArray(appBackupIds)); + assert(Array.isArray(backupIds)); assert.strictEqual(typeof callback, 'function'); - async.each([backupId].concat(appBackupIds), function (id, callback) { + async.eachSeries(backupIds, function (id, callback) { var filePath = getBackupFilePath(apiConfig, id); - fs.unlink(filePath, function (error) { - if (error) console.error('Unable to remove %s. Not fatal.', filePath, error); - callback(); - }); + if (!safe.fs.unlinkSync(filePath)) { + debug('removeBackups: Unable to remove %s : %s', filePath, safe.error.message); + return callback(); + } + + safe.fs.rmdirSync(path.dirname(filePath)); // try to cleanup empty directories + + callback(); + }, callback); } diff --git a/src/storage/interface.js b/src/storage/interface.js index 49b4695ae..868c46d03 100644 --- a/src/storage/interface.js +++ b/src/storage/interface.js @@ -10,7 +10,7 @@ exports = module.exports = { backup: backup, restore: restore, copyBackup: copyBackup, - removeBackup: removeBackup, + removeBackups: removeBackups, backupDone: backupDone, @@ -52,10 +52,9 @@ function copyBackup(apiConfig, oldBackupId, newBackupId, callback) { callback(new Error('not implemented')); } -function removeBackup(apiConfig, backupId, appBackupIds, callback) { +function removeBackups(apiConfig, backupIds, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert.strictEqual(typeof backupId, 'string'); - assert(Array.isArray(appBackupIds)); + assert(Array.isArray(backupIds)); assert.strictEqual(typeof callback, 'function'); // Result: none diff --git a/src/storage/s3.js b/src/storage/s3.js index 33360c97d..50899d072 100644 --- a/src/storage/s3.js +++ b/src/storage/s3.js @@ -4,7 +4,7 @@ exports = module.exports = { backup: backup, restore: restore, copyBackup: copyBackup, - removeBackup: removeBackup, + removeBackups: removeBackups, backupDone: backupDone, @@ -167,10 +167,9 @@ function copyBackup(apiConfig, oldBackupId, newBackupId, callback) { }); } -function removeBackup(apiConfig, backupId, appBackupIds, callback) { +function removeBackups(apiConfig, backupIds, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert.strictEqual(typeof backupId, 'string'); - assert(Array.isArray(appBackupIds)); + assert(Array.isArray(backupIds)); assert.strictEqual(typeof callback, 'function'); getBackupCredentials(apiConfig, function (error, credentials) { @@ -178,11 +177,17 @@ function removeBackup(apiConfig, backupId, appBackupIds, callback) { var params = { Bucket: apiConfig.bucket, - Key: getBackupFilePath(apiConfig, backupId) + Delete: { + Objects: [ ] // { Key } + } }; + backupIds.forEach(function (backupId) { + params.Delete.Objects.push({ Key: getBackupFilePath(apiConfig, backupId) }); + }); + var s3 = new AWS.S3(credentials); - s3.deleteObject(params, function (error) { + s3.deleteObjects(params, function (error) { if (error) console.error('Unable to remove %s. Not fatal.', params.Key, error); callback(null); }); diff --git a/src/test/storage-test.js b/src/test/storage-test.js index 9e1193716..68ef910ee 100644 --- a/src/test/storage-test.js +++ b/src/test/storage-test.js @@ -185,7 +185,7 @@ describe('Storage', function () { it('can remove backup', function (done) { // will be verified with next test trying to restore the removed one - filesystem.removeBackup(gBackupConfig, gBackupId_1, [], done); + filesystem.removeBackups(gBackupConfig, [ gBackupId_1 ], done); }); it('cannot restore deleted backup', function (done) { @@ -214,7 +214,7 @@ describe('Storage', function () { }); it('can remove backup copy', function (done) { - filesystem.removeBackup(gBackupConfig, gBackupId_2, [], done); + filesystem.removeBackups(gBackupConfig, [ gBackupId_2 ], done); }); }); @@ -305,7 +305,7 @@ describe('Storage', function () { it('can remove backup', function (done) { // will be verified with next test trying to restore the removed one - s3.removeBackup(gBackupConfig, gBackupId_1, [], done); + s3.removeBackups(gBackupConfig, [ gBackupId_1 ], done); }); it('cannot restore deleted backup', function (done) { @@ -334,7 +334,7 @@ describe('Storage', function () { }); it('can remove backup copy', function (done) { - s3.removeBackup(gBackupConfig, gBackupId_2, [], done); + s3.removeBackups(gBackupConfig, [ gBackupId_2 ], done); }); }); }); diff --git a/webadmin/src/views/settings.html b/webadmin/src/views/settings.html index 99523bbe3..de05ae791 100644 --- a/webadmin/src/views/settings.html +++ b/webadmin/src/views/settings.html @@ -145,6 +145,8 @@