From b1b6f70118d87c0ee2a2caa963b07a8cea286049 Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Thu, 6 Aug 2020 22:04:46 -0700 Subject: [PATCH] Kill all tasks on shutdown and startup BindsTo will kill all the tasks when systemctl stop box is executed. But when restarted, it keeps the tasks running. Because of this behavior, we kill the tasks on startup and stop of the box code. --- setup/start/sudoers | 2 +- src/apptask.js | 2 ++ src/cloudron.js | 5 ++++- src/platform.js | 4 ++-- src/scripts/clearvolume.sh | 2 +- src/scripts/starttask.sh | 33 ++++++++++++++++----------------- src/scripts/stoptask.sh | 10 +++++++--- src/tasks.js | 12 +++++++----- src/test/checkInstall | 4 +++- 9 files changed, 43 insertions(+), 31 deletions(-) diff --git a/setup/start/sudoers b/setup/start/sudoers index 4c714c74b..94e70e13f 100644 --- a/setup/start/sudoers +++ b/setup/start/sudoers @@ -54,7 +54,7 @@ Defaults!/home/yellowtent/box/src/scripts/rmmailbox.sh env_keep="HOME BOX_ENV" yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/rmmailbox.sh Defaults!/home/yellowtent/box/src/scripts/starttask.sh env_keep="HOME BOX_ENV" -yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/starttask.sh +yellowtent ALL=(root) NOPASSWD:SETENV: /home/yellowtent/box/src/scripts/starttask.sh Defaults!/home/yellowtent/box/src/scripts/stoptask.sh env_keep="HOME BOX_ENV" yellowtent ALL=(root) NOPASSWD: /home/yellowtent/box/src/scripts/stoptask.sh diff --git a/src/apptask.js b/src/apptask.js index 480005af9..91ffdd8f4 100644 --- a/src/apptask.js +++ b/src/apptask.js @@ -1063,6 +1063,8 @@ function run(appId, args, progressCallback, callback) { return stop(app, args, progressCallback, callback); case apps.ISTATE_PENDING_RESTART: return restart(app, args, progressCallback, callback); + case apps.ISTATE_INSTALLED: // can only happen when we have a bug in our code while testing/development + return updateApp(app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null }, callback); default: debugApp(app, 'apptask launched with invalid command'); return callback(new BoxError(BoxError.INTERNAL_ERROR, 'Unknown install command in apptask:' + app.installationState)); diff --git a/src/cloudron.js b/src/cloudron.js index dd2242cad..316a6684b 100644 --- a/src/cloudron.js +++ b/src/cloudron.js @@ -66,7 +66,7 @@ function uninitialize(callback) { async.series([ cron.stopJobs, - platform.stop + platform.stopAllTasks ], callback); } @@ -109,6 +109,9 @@ function notifyUpdate(callback) { // each of these tasks can fail. we will add some routes to fix/re-run them function runStartupTasks() { + // stop all the systemd tasks + platform.stopAllTasks(NOOP_CALLBACK); + // configure nginx to be reachable by IP reverseProxy.writeDefaultConfig(NOOP_CALLBACK); diff --git a/src/platform.js b/src/platform.js index 3a698350c..13cc58da6 100644 --- a/src/platform.js +++ b/src/platform.js @@ -2,7 +2,7 @@ exports = module.exports = { start: start, - stop: stop, + stopAllTasks: stopAllTasks, // exported for testing _isReady: false @@ -73,7 +73,7 @@ function start(callback) { }); } -function stop(callback) { +function stopAllTasks(callback) { tasks.stopAllTasks(callback); } diff --git a/src/scripts/clearvolume.sh b/src/scripts/clearvolume.sh index b05ed6783..6f7a075a8 100755 --- a/src/scripts/clearvolume.sh +++ b/src/scripts/clearvolume.sh @@ -37,4 +37,4 @@ if [[ "${cmd}" == "clear" ]]; then else # this make not succeed if volume is a mount point rmdir "${volume_dir}" || true -fi \ No newline at end of file +fi diff --git a/src/scripts/starttask.sh b/src/scripts/starttask.sh index dd7e417f6..a5d03094d 100755 --- a/src/scripts/starttask.sh +++ b/src/scripts/starttask.sh @@ -2,6 +2,9 @@ set -eu -o pipefail +readonly script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly task_worker="${script_dir}/../taskworker.js" + if [[ ${EUID} -ne 0 ]]; then echo "This script should be run as root." > /dev/stderr exit 1 @@ -21,24 +24,20 @@ readonly task_id="$1" readonly logfile="$2" readonly nice="$3" -readonly id=$(id -u yellowtent) readonly service_name="cloudron-task-${task_id}" - systemctl reset-failed "${service_name}" || true -# keep the env vars in sync with box.service -systemd-run --unit "${service_name}" \ - -p BindsTo=box.service \ - --pipe \ - --wait \ - --property=OOMScoreAdjust=-1000 \ - --nice "${nice}" \ - --uid=${id} \ - --gid=${id} \ - -E HOME=/home/yellowtent \ - -E USER=yellowtent \ - -E DEBUG=box:* \ - -E BOX_ENV=cloudron \ - -E NODE_ENV=production \ - /home/yellowtent/box/src/taskworker.js "${task_id}" "${logfile}" +readonly id=$(id -u $SUDO_USER) + +# Note: BindsTo will kill this task when the box is stopped. but will not kill this task when restarted! +# For this reason, we have code to kill the tasks both on shutdown and startup +if [[ "$BOX_ENV" == "cloudron" ]]; then + binds_to="-p BindsTo=box.service" +else + binds_to="" +fi + +systemd-run --unit "${service_name}" --pipe --wait --property=OOMScoreAdjust=-1000 --nice "${nice}" --uid=${id} --gid=${id} \ + ${binds_to} -E HOME=${HOME} -E USER=${SUDO_USER} -E DEBUG=${DEBUG} -E BOX_ENV=${BOX_ENV} -E NODE_ENV=production \ + "${task_worker}" "${task_id}" "${logfile}" diff --git a/src/scripts/stoptask.sh b/src/scripts/stoptask.sh index 5d67b6833..867a0ab3b 100755 --- a/src/scripts/stoptask.sh +++ b/src/scripts/stoptask.sh @@ -19,7 +19,11 @@ fi task_id="$1" -service_name="cloudron-task-${task_id}" - -systemctl kill --signal=SIGTERM "${service_name}" +if [[ "${task_id}" == "all" ]]; then + systemctl list-units --full --no-legend cloudron-task-* # just to show who was running + systemctl kill --signal=SIGTERM cloudron-task-* || true +else + readonly service_name="cloudron-task-${task_id}" + systemctl kill --signal=SIGTERM "${service_name}" || true +fi diff --git a/src/tasks.js b/src/tasks.js index f28224025..026324e6e 100644 --- a/src/tasks.js +++ b/src/tasks.js @@ -38,7 +38,6 @@ exports = module.exports = { }; let assert = require('assert'), - async = require('async'), BoxError = require('./boxerror.js'), debug = require('debug')('box:tasks'), path = require('path'), @@ -144,7 +143,9 @@ function startTask(id, options, callback) { let killTimerId = null, timedOut = false; - shell.sudo('startTask', [ START_TASK_CMD, id, logFile, options.nice || 0 ], {}, function (error) { + gTasks[id] = shell.sudo('startTask', [ START_TASK_CMD, id, logFile, options.nice || 0 ], { preserveEnv: true }, function (error) { + if (!gTasks[id]) return; // ignore task exit since we are shutting down. see stopAllTasks + const code = error ? error.code : 0; const signal = error ? error.signal : 0; @@ -200,9 +201,10 @@ function stopTask(id, callback) { function stopAllTasks(callback) { assert.strictEqual(typeof callback, 'function'); - async.eachSeries(Object.keys(gTasks), function (id, iteratorDone) { - stopTask(id, () => iteratorDone()); // ignore any error - }, callback); + debug('stopTask: stopping all tasks'); + + gTasks = {}; // this signals startTask() to not set completion status as "crashed" + shell.sudo('stopTask', [ STOP_TASK_CMD, 'all' ], {}, callback); } function listByTypePaged(type, page, perPage, callback) { diff --git a/src/test/checkInstall b/src/test/checkInstall index cacf1d0cf..79b45bae3 100755 --- a/src/test/checkInstall +++ b/src/test/checkInstall @@ -22,7 +22,9 @@ scripts=("${SOURCE_DIR}/src/scripts/clearvolume.sh" \ "${SOURCE_DIR}/src/scripts/collectlogs.sh" \ "${SOURCE_DIR}/src/scripts/configurecollectd.sh" \ "${SOURCE_DIR}/src/scripts/remotesupport.sh" \ - "${SOURCE_DIR}/src/scripts/backupupload.js" \ + "${SOURCE_DIR}/src/scripts/starttask.sh" \ + "${SOURCE_DIR}/src/scripts/stoptask.sh" \ + "${SOURCE_DIR}/src/scripts/rmmailbox.sh" \ "${SOURCE_DIR}/src/scripts/configurelogrotate.sh") for script in "${scripts[@]}"; do