collectd: cache du values and send it every Interval (20)

collectd plugin ordering matters. the write_graphite plugin establishes
a TCP connection but there is a race between that and the df/du values that
get reported. du is especially problematic since we report this only every 12 hours.

so, instead we cache the values and report it every 20 seconds. on the carbon side,
it will just retain every 12 hours (since that is the whisper retention period).

there is also FlushInterval which I am not 100% sure has any effect. by default, the
write_graphite plugin waits for 1428 bytes to be accumulated. (https://manpages.debian.org/unstable/collectd-core/collectd.conf.5.en.html)

https://github.com/collectd/collectd/issues/2672
https://github.com/collectd/collectd/pull/1044

I found this syntax hidden deep inside https://www.cisco.com/c/en/us/td/docs/net_mgmt/virtual_topology_system/2_6_3/user_guide/Cisco_VTS_2_6_3_User_Guide/Cisco_VTS_2_6_1_User_Guide_chapter_01111.pdf

(cherry picked from commit c1ee3dcbd4)

CHANGES

@@ -2232,3 +2232,17 @@
 [6.2.4]
 * Another addon crash fix
 
+[6.2.5]
+* update: set memory limit properly
+* Fix bug where renew certs button did not work
+* sftp: fix rebuild condition
+* Fix display of user management/dashboard visiblity for email apps
+* graphite: disable tagdb and reduce log noise
+
+[6.2.6]
+* Fix issue where collectd is restarted too quickly before graphite
+
+[6.2.7]
+* collectd: restart on graphite upgrade
+
+* 

setup/start/collectd/collectd.conf

@@ -164,7 +164,9 @@ LoadPlugin swap
 #LoadPlugin vmem
 #LoadPlugin vserver
 #LoadPlugin wireless
-LoadPlugin write_graphite
+<LoadPlugin write_graphite>
+   FlushInterval 20
+</LoadPlugin>
 #LoadPlugin write_http
 #LoadPlugin write_riemann
 

setup/start/collectd/du.py

@@ -6,19 +6,26 @@ PATHS = [] # { name, dir, exclude }
 # there is a pattern in carbon/storage-schemas.conf which stores values every 12h for a year
 INTERVAL = 60 * 60 * 12 # twice a day. change values in docker-graphite if you change this
 
+# we used to pass the INTERVAL as a parameter to register_read. however, collectd write_graphite
+# takes a bit to load (tcp connection) and drops the du data. this then means that we have to wait
+# for INTERVAL secs for du data. instead, we just cache the value for INTERVAL instead
+CACHE = dict()
+CACHE_TIME = 0
+
 def du(pathinfo):
     # -B1 makes du print block sizes and not apparent sizes (to match df which also uses block sizes)
-    cmd = 'timeout 1800 du -DsB1 "{}"'.format(pathinfo['dir'])
+    dirname = pathinfo['dir']
+    cmd = 'timeout 1800 du -DsB1 "{}"'.format(dirname)
     if pathinfo['exclude'] != '':
         cmd += ' --exclude "{}"'.format(pathinfo['exclude'])
 
     collectd.info('computing size with command: %s' % cmd);
     try:
         size = subprocess.check_output(cmd, shell=True).split()[0].decode('utf-8')
-        collectd.info('\tsize of %s is %s (time: %i)' % (pathinfo['dir'], size, int(time.time())))
+        collectd.info('\tsize of %s is %s (time: %i)' % (dirname, size, int(time.time())))
         return size
     except Exception as e:
-        collectd.info('\terror getting the size of %s: %s' % (pathinfo['dir'], str(e)))
+        collectd.info('\terror getting the size of %s: %s' % (dirname, str(e)))
         return 0
 
 def parseSize(size):
@@ -64,19 +71,35 @@ def init():
     collectd.info('custom du plugin initialized with %s %s' % (PATHS, sys.version))
 
 def read():
+    global CACHE, CACHE_TIME
+
+    # read from cache if < 12 hours
+    read_cache = (time.time() - CACHE_TIME) < INTERVAL
+
+    if not read_cache:
+        CACHE_TIME = time.time()
+
     for pathinfo in PATHS:
-        size = du(pathinfo)
+        dirname = pathinfo['dir']
+        if read_cache and dirname in CACHE:
+            size = CACHE[dirname]
+        else:
+            size = du(pathinfo)
+            CACHE[dirname] = size
 
         # type comes from https://github.com/collectd/collectd/blob/master/src/types.db
         val = collectd.Values(type='capacity', plugin='du', plugin_instance=pathinfo['name'])
         val.dispatch(values=[size], type_instance='usage')
 
-    size = dockerSize()
+    if read_cache and 'docker' in CACHE:
+        size = CACHE['docker']
+    else:
+        size = dockerSize()
+        CACHE['docker'] = size
+
     val = collectd.Values(type='capacity', plugin='du', plugin_instance='docker')
     val.dispatch(values=[size], type_instance='usage')
 
-
-
 collectd.register_init(init)
 collectd.register_config(configure)
-collectd.register_read(read, INTERVAL)
+collectd.register_read(read)

setup/start/sudoers

@@ -13,9 +13,6 @@ yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/mkdirvolume.sh
 Defaults!/home/yellowtent/box/src/scripts/rmaddondir.sh env_keep="HOME BOX_ENV"
 yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/rmaddondir.sh
 
-Defaults!/home/yellowtent/box/src/scripts/reloadnginx.sh env_keep="HOME BOX_ENV"
-yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/reloadnginx.sh
-
 Defaults!/home/yellowtent/box/src/scripts/reboot.sh env_keep="HOME BOX_ENV"
 yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/reboot.sh
 
@@ -41,11 +38,8 @@ yellowtent ALL=(root) NOPASSWD:SETENV: /home/yellowtent/box/src/scripts/backupup
 Defaults!/home/yellowtent/box/src/scripts/restart.sh env_keep="HOME BOX_ENV"
 yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/restart.sh
 
-Defaults!/home/yellowtent/box/src/scripts/restartdocker.sh env_keep="HOME BOX_ENV"
-yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/restartdocker.sh
-
-Defaults!/home/yellowtent/box/src/scripts/restartunbound.sh env_keep="HOME BOX_ENV"
-yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/restartunbound.sh
+Defaults!/home/yellowtent/box/src/scripts/restartservice.sh env_keep="HOME BOX_ENV"
+yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/restartservice.sh
 
 Defaults!/home/yellowtent/box/src/scripts/rmmailbox.sh env_keep="HOME BOX_ENV"
 yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/rmmailbox.sh

src/apps.js

@@ -629,7 +629,7 @@ function scheduleTask(appId, installationState, taskId, callback) {
         if (error) return callback(error);
 
         let memoryLimit = 400;
-        if (installationState === exports.ISTATE_PENDING_BACKUP || installationState === exports.ISTATE_PENDING_CLONE || installationState === exports.ISTATE_PENDING_RESTORE) {
+        if (installationState === exports.ISTATE_PENDING_BACKUP || installationState === exports.ISTATE_PENDING_CLONE || installationState === exports.ISTATE_PENDING_RESTORE || installationState === exports.ISTATE_PENDING_UPDATE) {
             memoryLimit = 'memoryLimit' in backupConfig ? Math.max(backupConfig.memoryLimit/1024/1024, 400) : 400;
         }
 

src/apptaskmanager.js

@@ -79,14 +79,16 @@ function scheduleTask(appId, taskId, options, callback) {
         delete gActiveTasks[appId];
         locker.unlock(locker.OP_APPTASK); // unlock event will trigger next task
 
-        // post app task hooks
-        services.rebuildService('sftp', error => { if (error) debug('Unable to rebuild sftp:', error); });
         scheduler.resumeJobs(appId);
     });
 }
 
 function startNextTask() {
-    if (gPendingTasks.length === 0) return;
+    if (gPendingTasks.length === 0) {
+        // rebuild sftp when task queue is empty. this minimizes risk of sftp rebuild overlapping with other app tasks
+        services.rebuildService('sftp', error => { if (error) debug('Unable to rebuild sftp:', error); });
+        return;
+    }
 
     assert(Object.keys(gActiveTasks).length < TASK_CONCURRENCY);
 

src/cert/acme2.js

@@ -572,18 +572,20 @@ Acme2.prototype.acmeFlow = function (hostname, domain, callback) {
 Acme2.prototype.getDirectory = function (callback) {
     const that = this;
 
-    request.get(this.caDirectory, { json: true, timeout: 30000 }, function (error, response) {
-        if (error) return callback(new BoxError(BoxError.NETWORK_ERROR, `Network error getting directory: ${error.message}`));
-        if (response.statusCode !== 200) return callback(new BoxError(BoxError.EXTERNAL_ERROR, 'Invalid response code when fetching directory : ' + response.statusCode));
+    async.retry({ times: 3, interval: 20000 }, function (retryCallback) {
+        request.get(that.caDirectory, { json: true, timeout: 30000 }, function (error, response) {
+            if (error) return retryCallback(new BoxError(BoxError.NETWORK_ERROR, `Network error getting directory: ${error.message}`));
+            if (response.statusCode !== 200) return retryCallback(new BoxError(BoxError.EXTERNAL_ERROR, 'Invalid response code when fetching directory : ' + response.statusCode));
 
-        if (typeof response.body.newNonce !== 'string' ||
-            typeof response.body.newOrder !== 'string' ||
-            typeof response.body.newAccount !== 'string') return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Invalid response body : ${response.body}`));
+            if (typeof response.body.newNonce !== 'string' ||
+                typeof response.body.newOrder !== 'string' ||
+                typeof response.body.newAccount !== 'string') return retryCallback(new BoxError(BoxError.EXTERNAL_ERROR, `Invalid response body : ${response.body}`));
 
-        that.directory = response.body;
+            that.directory = response.body;
 
-        callback(null);
-    });
+            retryCallback(null);
+        });
+    }, callback);
 };
 
 Acme2.prototype.getCertificate = function (vhost, domain, callback) {

src/graphite.js

@@ -1,50 +0,0 @@
-'use strict';
-
-exports = module.exports = {
-    start,
-
-    DEFAULT_MEMORY_LIMIT: 256 * 1024 * 1024
-};
-
-var assert = require('assert'),
-    async = require('async'),
-    infra = require('./infra_version.js'),
-    paths = require('./paths.js'),
-    shell = require('./shell.js'),
-    system = require('./system.js');
-
-function start(existingInfra, serviceConfig, callback) {
-    assert.strictEqual(typeof existingInfra, 'object');
-    assert.strictEqual(typeof serviceConfig, 'object');
-    assert.strictEqual(typeof callback, 'function');
-
-    const tag = infra.images.graphite.tag;
-    const dataDir = paths.PLATFORM_DATA_DIR;
-    const memoryLimit = serviceConfig.memoryLimit || exports.DEFAULT_MEMORY_LIMIT;
-    const memory = system.getMemoryAllocation(memoryLimit);
-
-    const cmd = `docker run --restart=always -d --name="graphite" \
-                --hostname graphite \
-                --net cloudron \
-                --net-alias graphite \
-                --log-driver syslog \
-                --log-opt syslog-address=udp://127.0.0.1:2514 \
-                --log-opt syslog-format=rfc5424 \
-                --log-opt tag=graphite \
-                -m ${memory} \
-                --memory-swap ${memoryLimit} \
-                --dns 172.18.0.1 \
-                --dns-search=. \
-                -p 127.0.0.1:2003:2003 \
-                -p 127.0.0.1:2004:2004 \
-                -p 127.0.0.1:8417:8000 \
-                -v "${dataDir}/graphite:/var/lib/graphite" \
-                --label isCloudronManaged=true \
-                --read-only -v /tmp -v /run "${tag}"`;
-
-    async.series([
-        shell.exec.bind(null, 'stopGraphite', 'docker stop graphite || true'),
-        shell.exec.bind(null, 'removeGraphite', 'docker rm -f graphite || true'),
-        shell.exec.bind(null, 'startGraphite', cmd)
-    ], callback);
-}

src/infra_version.js

@@ -21,7 +21,7 @@ exports = module.exports = {
         'mongodb': { repo: 'cloudron/mongodb', tag: 'cloudron/mongodb:4.0.1@sha256:ad20a9a5dcb2ab132374a7c8d44b89af0ec37651cf889e570f7625b02ee85fdf' },
         'redis': { repo: 'cloudron/redis', tag: 'cloudron/redis:3.0.2@sha256:caaa1f7f4055ae8990d8ec65bd100567496df7e4ed5eb427867f3717a8dcbf92' },
         'mail': { repo: 'cloudron/mail', tag: 'cloudron/mail:3.2.3@sha256:fdc4aa6d2c85aeafe65eaa4243aada0cc2e57b94f6eaee02c9b1a8fb89b01dd7' },
-        'graphite': { repo: 'cloudron/graphite', tag: 'cloudron/graphite:2.4.0@sha256:953bbd8b72a9108a8526d2c0bdbba67e1e1563ff59d0a117f0884dba1576f3dd' },
+        'graphite': { repo: 'cloudron/graphite', tag: 'cloudron/graphite:3.0.0@sha256:b00b64b8df4032985d7a1ddd548a2713b6d7d88a54ebe9b7d324cece2bd6829e' },
         'sftp': { repo: 'cloudron/sftp', tag: 'cloudron/sftp:3.2.0@sha256:61e8247ded1e07cf882ca478dab180960357c614472e80b938f1f690a46788c2' }
     }
 };

src/reverseproxy.js

@@ -54,8 +54,8 @@ var acme2 = require('./cert/acme2.js'),
     users = require('./users.js'),
     util = require('util');
 
-var NGINX_APPCONFIG_EJS = fs.readFileSync(__dirname + '/nginxconfig.ejs', { encoding: 'utf8' }),
-    RELOAD_NGINX_CMD = path.join(__dirname, 'scripts/reloadnginx.sh');
+const NGINX_APPCONFIG_EJS = fs.readFileSync(__dirname + '/nginxconfig.ejs', { encoding: 'utf8' });
+const RESTART_SERVICE_CMD = path.join(__dirname, 'scripts/restartservice.sh');
 
 function nginxLocation(s) {
     if (!s.startsWith('!')) return s;
@@ -172,7 +172,7 @@ function validateCertificate(location, domainObject, certificate) {
 function reload(callback) {
     if (constants.TEST) return callback();
 
-    shell.sudo('reload', [ RELOAD_NGINX_CMD ], {}, function (error) {
+    shell.sudo('reload', [ RESTART_SERVICE_CMD, 'nginx' ], {}, function (error) {
         if (error) return callback(new BoxError(BoxError.NGINX_ERROR, `Error reloading nginx: ${error.message}`));
 
         callback();

src/scripts/reloadnginx.sh

@@ -1,17 +0,0 @@
-#!/bin/bash
-
-set -eu -o pipefail
-
-if [[ ${EUID} -ne 0 ]]; then
-    echo "This script should be run as root." > /dev/stderr
-    exit 1
-fi
-
-if [[ $# == 1 && "$1" == "--check" ]]; then
-    echo "OK"
-    exit 0
-fi
-
-if [[ "${BOX_ENV}" == "cloudron" ]]; then
-    nginx -s reload
-fi

src/scripts/restartdocker.sh

@@ -1,18 +0,0 @@
-#!/bin/bash
-
-set -eu -o pipefail
-
-if [[ ${EUID} -ne 0 ]]; then
-    echo "This script should be run as root." > /dev/stderr
-    exit 1
-fi
-
-if [[ $# == 1 && "$1" == "--check" ]]; then
-    echo "OK"
-    exit 0
-fi
-
-if [[ "${BOX_ENV}" == "cloudron" ]]; then
-    systemctl restart docker
-fi
-

src/scripts/restartservice.sh

@@ -0,0 +1,37 @@
+#!/bin/bash
+
+set -eu -o pipefail
+
+if [[ ${EUID} -ne 0 ]]; then
+    echo "This script should be run as root." > /dev/stderr
+    exit 1
+fi
+
+if [[ $# -eq 0 ]]; then
+    echo "No arguments supplied"
+    exit 1
+fi
+
+if [[ "$1" == "--check" ]]; then
+    echo "OK"
+    exit 0
+fi
+
+[[ "${BOX_ENV}" != "cloudron" ]] && exit
+
+service="$1"
+
+if [[ "${service}" == "unbound" ]]; then
+    unbound-anchor -a /var/lib/unbound/root.key
+    systemctl restart unbound
+elif [[ "${service}" == "nginx" ]]; then
+    nginx -s reload
+elif [[ "${service}" == "docker" ]]; then
+    systemctl restart docker
+elif [[ "${service}" == "collectd" ]]; then
+    systemctl restart collectd
+else
+    echo "Unknown service ${service}"
+    exit 1
+fi
+

src/scripts/restartunbound.sh

@@ -1,19 +0,0 @@
-#!/bin/bash
-
-set -eu -o pipefail
-
-if [[ ${EUID} -ne 0 ]]; then
-    echo "This script should be run as root." > /dev/stderr
-    exit 1
-fi
-
-if [[ $# == 1 && "$1" == "--check" ]]; then
-    echo "OK"
-    exit 0
-fi
-
-if [[ "${BOX_ENV}" == "cloudron" ]]; then
-    unbound-anchor -a /var/lib/unbound/root.key
-    systemctl restart unbound
-fi
-

src/scripts/rmaddondir.sh

@@ -19,13 +19,17 @@ fi
 
 addon="$1"
 appid="${2:-}"   # only valid for redis
-if [[ "${addon}" != "postgresql" && "${addon}" != "mysql" && "${addon}" != "mongodb" && "${addon}" != "redis" ]]; then
-    echo "${addon} must be postgresql/mysql/mongodb/redis"
+if [[ "${addon}" != "postgresql" && "${addon}" != "mysql" && "${addon}" != "mongodb" && "${addon}" != "redis" && "${addon}" != "graphite" ]]; then
+    echo "${addon} must be postgresql/mysql/mongodb/redis/graphite"
     exit 1
 fi
 
 if [[ "${BOX_ENV}" == "cloudron" ]]; then
-    readonly addon_dir="${HOME}/platformdata/${addon}/${appid}"
+    if [[ "${addon}" == "graphite" ]]; then
+        readonly addon_dir="${HOME}/platformdata/graphite"
+    else
+        readonly addon_dir="${HOME}/platformdata/${addon}/${appid}"
+    fi
 else
     readonly addon_dir="${HOME}/.cloudron_test/platformdata/${addon}/${appid}"
 fi

src/services.js

@@ -41,7 +41,6 @@ var appdb = require('./appdb.js'),
     debug = require('debug')('box:services'),
     docker = require('./docker.js'),
     fs = require('fs'),
-    graphite = require('./graphite.js'),
     hat = require('./hat.js'),
     infra = require('./infra_version.js'),
     mail = require('./mail.js'),
@@ -64,6 +63,7 @@ var appdb = require('./appdb.js'),
 const NOOP = function (app, options, callback) { return callback(); };
 const NOOP_CALLBACK = function (error) { if (error) debug(error); };
 const RMADDONDIR_CMD = path.join(__dirname, 'scripts/rmaddondir.sh');
+const RESTART_SERVICE_CMD = path.join(__dirname, 'scripts/restartservice.sh');
 
 // setup can be called multiple times for the same app (configure crash restart) and existing data must not be lost
 // teardown is destructive. app data stored with the addon is lost
@@ -219,8 +219,8 @@ const SERVICES = {
     },
     graphite: {
         status: statusGraphite,
-        restart: docker.restartContainer.bind(null, 'graphite'),
-        defaultMemoryLimit: graphite.DEFAULT_MEMORY_LIMIT
+        restart: restartGraphite,
+        defaultMemoryLimit: 256 * 1024 * 1024
     },
     nginx: {
         status: statusNginx,
@@ -541,7 +541,7 @@ function rebuildService(id, callback) {
         if (id === 'postgresql') return startPostgresql({ version: 'none' }, callback);
         if (id === 'mysql') return startMysql({ version: 'none' }, callback);
         if (id === 'sftp') return sftp.rebuild(serviceConfig, { /* options */ }, callback);
-        if (id === 'graphite') return graphite.start({ version: 'none' }, serviceConfig, callback);
+        if (id === 'graphite') return startGraphite({ version: 'none' }, serviceConfig, callback);
 
         // nothing to rebuild for now.
         // TODO: mongo/postgresql/mysql need to be scaled down.
@@ -845,7 +845,7 @@ function startServices(existingInfra, callback) {
                 startPostgresql.bind(null, existingInfra),
                 startMongodb.bind(null, existingInfra),
                 startRedis.bind(null, existingInfra),
-                graphite.start.bind(null, existingInfra, servicesConfig['graphite'] || {}),
+                startGraphite.bind(null, existingInfra, servicesConfig['graphite'] || {}),
                 sftp.start.bind(null, existingInfra, servicesConfig['sftp'] || {}),
             );
         } else {
@@ -857,7 +857,7 @@ function startServices(existingInfra, callback) {
             if (infra.images.postgresql.tag !== existingInfra.images.postgresql.tag) startFuncs.push(startPostgresql.bind(null, existingInfra));
             if (infra.images.mongodb.tag !== existingInfra.images.mongodb.tag) startFuncs.push(startMongodb.bind(null, existingInfra));
             if (infra.images.redis.tag !== existingInfra.images.redis.tag) startFuncs.push(startRedis.bind(null, existingInfra));
-            if (infra.images.graphite.tag !== existingInfra.images.graphite.tag) startFuncs.push(graphite.start.bind(null, existingInfra, servicesConfig['graphite'] || {}));
+            if (infra.images.graphite.tag !== existingInfra.images.graphite.tag) startFuncs.push(startGraphite.bind(null, existingInfra, servicesConfig['graphite'] || {}));
             if (infra.images.sftp.tag !== existingInfra.images.sftp.tag) startFuncs.push(sftp.start.bind(null, existingInfra, servicesConfig['sftp'] || {}));
 
             debug('startServices: existing infra. incremental service create %j', startFuncs.map(function (f) { return f.name; }));
@@ -1781,6 +1781,54 @@ function restoreMongoDb(app, options, callback) {
     });
 }
 
+function startGraphite(existingInfra, serviceConfig, callback) {
+    assert.strictEqual(typeof existingInfra, 'object');
+    assert.strictEqual(typeof serviceConfig, 'object');
+    assert.strictEqual(typeof callback, 'function');
+
+    const tag = infra.images.graphite.tag;
+    const memoryLimit = serviceConfig.memoryLimit || 256 * 1024 * 1024;
+    const memory = system.getMemoryAllocation(memoryLimit);
+
+    const upgrading = existingInfra.version !== 'none' && requiresUpgrade(existingInfra.images.graphite.tag, tag);
+
+    if (upgrading) debug('startGraphite: graphite will be upgraded');
+
+    const cmd = `docker run --restart=always -d --name="graphite" \
+        --hostname graphite \
+        --net cloudron \
+        --net-alias graphite \
+        --log-driver syslog \
+        --log-opt syslog-address=udp://127.0.0.1:2514 \
+        --log-opt syslog-format=rfc5424 \
+        --log-opt tag=graphite \
+        -m ${memory} \
+        --memory-swap ${memoryLimit} \
+        --dns 172.18.0.1 \
+        --dns-search=. \
+        -p 127.0.0.1:2003:2003 \
+        -p 127.0.0.1:2004:2004 \
+        -p 127.0.0.1:8417:8000 \
+        -v "${paths.PLATFORM_DATA_DIR}/graphite:/var/lib/graphite" \
+        --label isCloudronManaged=true \
+        --read-only -v /tmp -v /run "${tag}"`;
+
+    async.series([
+        shell.exec.bind(null, 'stopGraphite', 'docker stop graphite || true'),
+        shell.exec.bind(null, 'removeGraphite', 'docker rm -f graphite || true'),
+        (done) => {
+            if (!upgrading) return done();
+            shell.sudo('removeGraphiteDir', [ RMADDONDIR_CMD, 'graphite' ], {}, done);
+        },
+        shell.exec.bind(null, 'startGraphite', cmd)
+    ], function (error) {
+        // restart collectd to get the disk stats after graphite starts. currently, there is no way to do graphite health check
+        if (!error) setTimeout(() => shell.sudo('restartcollectd', [ RESTART_SERVICE_CMD, 'collectd' ], {}, NOOP_CALLBACK), 60000);
+
+        callback(error);
+    });
+}
+
 function setupProxyAuth(app, options, callback) {
     assert.strictEqual(typeof app, 'object');
     assert.strictEqual(typeof options, 'object');
@@ -1820,6 +1868,10 @@ function startRedis(existingInfra, callback) {
             const redisName = 'redis-' + app.id;
 
             async.series([
+                (done) => {
+                    if (!upgrading) return done();
+                    backupRedis(app, {}, done);
+                },
                 shell.exec.bind(null, 'stopRedis', `docker stop ${redisName} || true`), // redis will backup as part of signal handling
                 shell.exec.bind(null, 'removeRedis', `docker rm -f ${redisName} || true`),
                 setupRedis.bind(null, app, app.manifest.addons.redis) // starts the container
@@ -2020,7 +2072,7 @@ function statusDocker(callback) {
 function restartDocker(callback) {
     assert.strictEqual(typeof callback, 'function');
 
-    shell.sudo('restartdocker', [ path.join(__dirname, 'scripts/restartdocker.sh') ], {}, NOOP_CALLBACK);
+    shell.sudo('restartdocker', [ RESTART_SERVICE_CMD, 'docker' ], {}, NOOP_CALLBACK);
 
     callback(null);
 }
@@ -2036,7 +2088,7 @@ function statusUnbound(callback) {
 function restartUnbound(callback) {
     assert.strictEqual(typeof callback, 'function');
 
-    shell.sudo('restartunbound', [ path.join(__dirname, 'scripts/restartunbound.sh') ], {}, NOOP_CALLBACK);
+    shell.sudo('restartunbound', [ RESTART_SERVICE_CMD, 'unbound' ], {}, NOOP_CALLBACK);
 
     callback(null);
 }
@@ -2052,7 +2104,7 @@ function statusNginx(callback) {
 function restartNginx(callback) {
     assert.strictEqual(typeof callback, 'function');
 
-    shell.sudo('reloadnginx', [ path.join(__dirname, 'scripts/reloadnginx.sh') ], {}, NOOP_CALLBACK);
+    shell.sudo('restartnginx', [ RESTART_SERVICE_CMD, 'nginx' ], {}, NOOP_CALLBACK);
 
     callback(null);
 }
@@ -2104,6 +2156,14 @@ function statusGraphite(callback) {
     });
 }
 
+function restartGraphite(callback) {
+    assert.strictEqual(typeof callback, 'function');
+
+    docker.restartContainer('graphite', callback);
+
+    setTimeout(() => shell.sudo('restartcollectd', [ RESTART_SERVICE_CMD, 'collectd' ], {}, NOOP_CALLBACK), 60000);
+}
+
 function teardownOauth(app, options, callback) {
     assert.strictEqual(typeof app, 'object');
     assert.strictEqual(typeof options, 'object');

src/sftp.js

@@ -21,24 +21,11 @@ var apps = require('./apps.js'),
     volumes = require('./volumes.js'),
     _ = require('underscore');
 
-var gRebuildInProgress = false;
 function rebuild(serviceConfig, options, callback) {
     assert.strictEqual(typeof serviceConfig, 'object');
     assert.strictEqual(typeof options, 'object');
     assert.strictEqual(typeof callback, 'function');
 
-    if (gRebuildInProgress) {
-        debug('waiting for other rebuild to finish');
-        return setTimeout(function () { rebuild(serviceConfig, options, callback); }, 5000);
-    }
-
-    gRebuildInProgress = true;
-
-    function done(error) {
-        gRebuildInProgress = false;
-        callback(error);
-    }
-
     debug('rebuilding container');
 
     const force = !!options.force;
@@ -48,7 +35,7 @@ function rebuild(serviceConfig, options, callback) {
     const cloudronToken = hat(8 * 128);
 
     apps.getAll(function (error, result) {
-        if (error) return done(error);
+        if (error) return callback(error);
 
         let dataDirs = [];
         result.forEach(function (app) {
@@ -89,7 +76,7 @@ function rebuild(serviceConfig, options, callback) {
 
                         if (!force && _.isEqual(currentDataDirs, dataDirs)) {
                             debug('Skipping rebuild, no changes');
-                            return done();
+                            return callback();
                         }
                     }
                 }
@@ -119,7 +106,7 @@ function rebuild(serviceConfig, options, callback) {
                     shell.exec.bind(null, 'stopSftp', 'docker stop sftp || true'),
                     shell.exec.bind(null, 'removeSftp', 'docker rm -f sftp || true'),
                     shell.exec.bind(null, 'startSftp', cmd)
-                ], done);
+                ], callback);
             });
         });
     });

src/storage/s3.js

@@ -157,7 +157,7 @@ function exists(apiConfig, backupFilePath, callback) {
             s3.headObject(params, function (error) {
                 if (!Object.keys(this.httpResponse.headers).some(h => h.startsWith('x-amz'))) return callback(new BoxError(BoxError.EXTERNAL_ERROR, 'not a s3 endpoint'));
                 if (error && S3_NOT_FOUND(error)) return callback(null, false);
-                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code));
+                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Error headObject ${backupFilePath}. Message: ${error.message} HTTP Code: ${error.code}`));
 
                 callback(null, true);
             });
@@ -169,7 +169,7 @@ function exists(apiConfig, backupFilePath, callback) {
             };
 
             s3.listObjects(listParams, function (error, listData) {
-                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code));
+                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Error listing objects ${backupFilePath}. Message: ${error.message} HTTP Code: ${error.code}`));
 
                 callback(null, listData.Contents.length !== 0);
             });
@@ -200,7 +200,7 @@ function download(apiConfig, backupFilePath, callback) {
                 ps.emit('error', new BoxError(BoxError.NOT_FOUND, `Backup not found: ${backupFilePath}`));
             } else {
                 debug(`download: ${apiConfig.bucket}:${backupFilePath} s3 stream error.`, error);
-                ps.emit('error', new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code)); // DO sets 'code'
+                ps.emit('error', new BoxError(BoxError.EXTERNAL_ERROR, `Error multipartDownload ${backupFilePath}. Message: ${error.message} HTTP Code: ${error.code}`));
             }
         });
 
@@ -231,7 +231,7 @@ function listDir(apiConfig, dir, batchSize, iteratorCallback, callback) {
 
         async.whilst((testDone) => testDone(null, !done), function listAndDownload(whilstCallback) {
             s3.listObjects(listParams, function (error, listData) {
-                if (error) return whilstCallback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code));
+                if (error) return whilstCallback(new BoxError(BoxError.EXTERNAL_ERROR, `Error listing objects in ${dir}. Message: ${error.message} HTTP Code: ${error.code}`));
 
                 if (listData.Contents.length === 0) { done = true; return whilstCallback(); }
 
@@ -505,7 +505,7 @@ function testConfig(apiConfig, callback) {
 
         var s3 = new AWS.S3(_.omit(credentials, 'retryDelayOptions', 'maxRetries'));
         s3.putObject(params, function (error) {
-            if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code)); // DO sets 'code'
+            if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Error put object cloudron-testfile. Message: ${error.message} HTTP Code: ${error.code}`));
 
             var params = {
                 Bucket: apiConfig.bucket,
@@ -513,7 +513,7 @@ function testConfig(apiConfig, callback) {
             };
 
             s3.deleteObject(params, function (error) {
-                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, error.message || error.code)); // DO sets 'code'
+                if (error) return callback(new BoxError(BoxError.EXTERNAL_ERROR, `Error del object cloudron-testfile. Message: ${error.message} HTTP Code: ${error.code}`));
 
                 callback();
             });

src/test/checkInstall

@@ -13,11 +13,9 @@ scripts=("${SOURCE_DIR}/src/scripts/clearvolume.sh" \
          "${SOURCE_DIR}/src/scripts/mvvolume.sh" \
          "${SOURCE_DIR}/src/scripts/mkdirvolume.sh" \
          "${SOURCE_DIR}/src/scripts/rmaddondir.sh" \
-         "${SOURCE_DIR}/src/scripts/reloadnginx.sh" \
          "${SOURCE_DIR}/src/scripts/reboot.sh" \
          "${SOURCE_DIR}/src/scripts/restart.sh" \
-         "${SOURCE_DIR}/src/scripts/restartdocker.sh" \
-         "${SOURCE_DIR}/src/scripts/restartunbound.sh" \
+         "${SOURCE_DIR}/src/scripts/restartservice.sh" \
          "${SOURCE_DIR}/src/scripts/update.sh" \
          "${SOURCE_DIR}/src/scripts/collectlogs.sh" \
          "${SOURCE_DIR}/src/scripts/configurecollectd.sh" \

src/test/shell-test.js

@@ -39,8 +39,8 @@ describe('shell', function () {
     });
 
     it('can sudo valid program', function (done) {
-        var RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/reloadnginx.sh');
-        shell.sudo('test', [ RELOAD_NGINX_CMD ], {}, function (error) {
+        var RELOAD_NGINX_CMD = path.join(__dirname, '../src/scripts/restartservice.sh');
+        shell.sudo('test', [ RELOAD_NGINX_CMD, 'nginx' ], {}, function (error) {
             expect(error).to.be.ok();
             done();
         });