From 28baef8929f0be2e1e0a7149caf046806690154b Mon Sep 17 00:00:00 2001 From: "girish@cloudron.io" Date: Fri, 15 Jan 2016 15:15:24 -0800 Subject: [PATCH] Go back to using docker exec in cloudron exec The main issue is that multiple cloudron exec sessions do not share the same rootfs. Which makes it annoying to debug. We also have some nginx timeout which drops you out of exec now and then resulting in loss of all state. --- src/apps.js | 43 ++++++++++++++++--------------------------- src/docker.js | 2 +- src/routes/apps.js | 3 --- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/src/apps.js b/src/apps.js index e4ad1c4c2..03dbd3bd3 100644 --- a/src/apps.js +++ b/src/apps.js @@ -648,42 +648,31 @@ function exec(appId, options, callback) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.NOT_FOUND, 'No such app')); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - var createOptions = { + var container = docker.connection.getContainer(app.containerId); + + var execOptions = { AttachStdin: true, AttachStdout: true, AttachStderr: true, - OpenStdin: true, - StdinOnce: false, - Tty: true + Tty: true, + Cmd: cmd }; - docker.createSubcontainer(app, app.id + '-exec-' + Date.now(), cmd, createOptions, function (error, container) { + container.exec(execOptions, function (error, exec) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - - container.attach({ stream: true, stdin: true, stdout: true, stderr: true }, function (error, stream) { + var startOptions = { + Detach: false, + Tty: true, + stdin: true // this is a dockerode option that enabled openStdin in the modem + }; + exec.start(startOptions, function(error, stream) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - docker.startContainer(container.id, function (error) { - if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); + if (options.rows && options.columns) { + exec.resize({ h: options.rows, w: options.columns }, function (error) { if (error) debug('Error resizing console', error); }); + } - if (options.rows && options.columns) { - container.resize({ h: options.rows, w: options.columns }, NOOP_CALLBACK); - } - - var deleteContainer = once(docker.deleteContainer.bind(null, container.id, NOOP_CALLBACK)); - - container.wait(function (error) { - if (error) debug('Error waiting on container', error); - - debug('exec: container finished', container.id); - - deleteContainer(); - }); - - stream.close = deleteContainer; - - callback(null, stream); - }); + return callback(null, stream); }); }); }); diff --git a/src/docker.js b/src/docker.js index 4e024560b..3276728c1 100644 --- a/src/docker.js +++ b/src/docker.js @@ -159,7 +159,7 @@ function createSubcontainer(app, name, cmd, options, callback) { var memoryLimit = manifest.memoryLimit || (developmentMode ? 0 : 1024 * 1024 * 200); // 200mb by default // for subcontainers, this should ideally be false. but docker does not allow network sharing if the app container is not running // this means cloudron exec does not work - var isolatedNetworkNs = isAppContainer || !developmentMode; + var isolatedNetworkNs = true; addons.getEnvironment(app, function (error, addonEnv) { if (error) return callback(new Error('Error getting addon environment : ' + error)); diff --git a/src/routes/apps.js b/src/routes/apps.js index 2c7cd040a..855d06989 100644 --- a/src/routes/apps.js +++ b/src/routes/apps.js @@ -369,8 +369,5 @@ function exec(req, res, next) { duplexStream.pipe(res.socket); res.socket.pipe(duplexStream); - - res.on('close', duplexStream.close); }); } -