This is an automated email from the ASF dual-hosted git repository. alexkli pushed a commit to branch containerkill in repository https://gitbox.apache.org/repos/asf/openwhisk-wskdebug.git
commit 3934f950685805d9a0656950e427c9bcf610f04d Author: Alexander Klimetschek <[email protected]> AuthorDate: Thu Jul 16 22:00:42 2020 -0700 detect if debug port is already used and remove left over container #59 --- package-lock.json | 5 +++ package.json | 1 + src/agentmgr.js | 16 +++++++++ src/dockerutils.js | 10 +++++- src/invoker.js | 60 +++++++++++++++++++++++++++++-- test/invoker.test.js | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 189 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index ec6eb41..c1f6510 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1660,6 +1660,11 @@ "resolved": "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz";, "integrity": "sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==" }, + "is-port-reachable": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/is-port-reachable/-/is-port-reachable-3.0.0.tgz";, + "integrity": "sha512-056IzLiWHdgVd6Eq1F9HtJl+cIkvi5X2MJ/A1fjQtByHkzQE1wGardnPhqrarOGDF88BOW+297X7PDvZ2vcyVg==" + }, "is-promise": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/is-promise/-/is-promise-2.1.0.tgz";, diff --git a/package.json b/package.json index cac0c22..9efd154 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "fetch-retry": "^3.1.0", "fs-extra": "^8.1.0", "get-port": "^5.1.1", + "is-port-reachable": "^3.0.0", "isomorphic-fetch": "^2.2.1", "livereload": "^0.9.1", "manakin": "^0.5.2", diff --git a/src/agentmgr.js b/src/agentmgr.js index 585d9e0..db33e62 100644 --- a/src/agentmgr.js +++ b/src/agentmgr.js @@ -413,6 +413,22 @@ class AgentMgr { // --------------------------------------< restoring >------------------ async restoreAction(isStartup) { + // if a restore is already running, wait for it to finish + if (this._restorePromise) { + await this._restorePromise; + return; + } + + // start actual restore and store the promise + this._restorePromise = this._restoreAction(isStartup); + // wait for the result + const result = await this._restorePromise; + // make sure to delete the promise once done + delete this._restorePromise; + return result; + } + + async _restoreAction(isStartup) { const copy = getActionCopyName(this.actionName); try { diff --git a/src/dockerutils.js b/src/dockerutils.js index 0298c63..7b1270f 100644 --- a/src/dockerutils.js +++ b/src/dockerutils.js @@ -66,7 +66,15 @@ function dockerRunArgs2CreateContainerConfig(args, containerConfig) { return containerConfig; } +function getContainerName(container) { + if (container.Names && container.Names.length >= 1) { + // remove leading slash + return container.Names[0].substring(1); + } +} + module.exports = { safeContainerName, - dockerRunArgs2CreateContainerConfig + dockerRunArgs2CreateContainerConfig, + getContainerName }; diff --git a/src/invoker.js b/src/invoker.js index f39c747..287a359 100644 --- a/src/invoker.js +++ b/src/invoker.js @@ -25,10 +25,12 @@ const Docker = require('dockerode'); const getPort = require('get-port'); const dockerUtils = require('./dockerutils'); const prettyBytes = require('pretty-bytes'); +const isPortReachable = require('is-port-reachable'); const RUNTIME_PORT = 8080; const MAX_INIT_RETRY_MS = 20000; // 20 sec const INIT_RETRY_DELAY_MS = 200; +const LABEL_ACTION_NAME = "org.apache.wskdebug.action"; // https://github.com/apache/incubator-openwhisk/blob/master/docs/reference.md#system-limits const OPENWHISK_DEFAULTS = { @@ -71,7 +73,7 @@ class OpenWhiskInvoker { this.wskProps = wskProps; this.wsk = wsk; - this.containerName = dockerUtils.safeContainerName(`wskdebug-${this.action.name}-${Date.now()}`); + this.containerName = dockerUtils.safeContainerName(`wskdebug-${this.actionName}-${Date.now()}`); this.docker = new Docker(); } @@ -105,6 +107,7 @@ class OpenWhiskInvoker { } } catch (e) { + console.log(e); log.warn("Could not retrieve runtime images from OpenWhisk, using default image list.", e.message); } return kinds.images[kind]; @@ -235,6 +238,47 @@ class OpenWhiskInvoker { }); } + getFullActionName() { + return `/${this.wskProps.namespace}/${this.actionName}`; + } + + async checkExistingContainers() { + // check if the debug port is already in use + if (await isPortReachable(this.debug.port)) { + + const containers = await this.docker.listContainers(); + const fullActionName = this.getFullActionName(); + + // then check if there is a left over container from a previous run with that port + for (const container of containers) { + for (const port of container.Ports) { + if (port.PublicPort === this.debug.port) { + // check if wskdebug container by looking at our label + if (container.Labels[LABEL_ACTION_NAME]) { + if (container.Labels[LABEL_ACTION_NAME] === fullActionName) { + // same action + log.warn(`Replacing container from a previous wskdebug run for this action (${dockerUtils.getContainerName(container)}).`) + const oldContainer = await this.docker.getContainer(container.Id); + await oldContainer.remove({force: true}); + return; + + } else { + // wskdebug of different action + throw new Error(`Debug port ${this.debug.port} already in use by wskdebug for action ${container.Labels[LABEL_ACTION_NAME]}, cotainer ${dockerUtils.getContainerName(container)} (id: ${container.Id}).`); + } + } else { + // some non-wskdebug container + throw new Error(`Debug port ${this.debug.port} already in use by another docker container ${dockerUtils.getContainerName(container)} (id: ${container.Id}).`); + } + } + } + } + + // some other process uses the port + throw new Error(`Debug port ${this.debug.port} already in use.`); + } + } + async startContainer(debug) { if (!await this.isImagePresent(this.image, debug)) { // show after 8 seconds, as VS code will timeout after 10 secs by default, @@ -262,6 +306,8 @@ class OpenWhiskInvoker { debug("Pull complete"); } + await this.checkExistingContainers(); + log.spinner('Starting container'); // links for docker create container config: @@ -279,6 +325,9 @@ class OpenWhiskInvoker { const createContainerConfig = { name: this.containerName, + Labels: { + [LABEL_ACTION_NAME]: this.getFullActionName() + }, Image: this.image, Cmd: [ 'sh', '-c', this.debug.command ], Env: [], @@ -427,7 +476,14 @@ class OpenWhiskInvoker { // log this here for VS Code, will be the last visible log message since // we will be killed by VS code after the container is gone after the kill() log.log(`Stopping container ${this.name()}.`); - await this.container.kill(); + try { + await this.container.remove({ force: true}); + } catch (e) { + // if we get a 404 the container is already gone (our goal), no need to log this error + if (e.statusCode !== 404) { + log.exception(e, "Error while removing container"); + } + } delete this.container; log.debug(`docker - stopped container ${this.name()}`); } diff --git a/test/invoker.test.js b/test/invoker.test.js new file mode 100644 index 0000000..4115b2d --- /dev/null +++ b/test/invoker.test.js @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* eslint-env mocha */ + +'use strict'; + +const OpenWhiskInvoker = require('../src/invoker'); +const Docker = require('dockerode'); +const assert = require("assert"); + +const ACTION_NAME = "myaction"; +const ACTION_METADATA = { + exec: { + kind: "nodejs" + }, + limits: { + } +}; +const WSK_PROPS = { + namespace: "namespace" +}; +const WSK = { + actions: { + client: { + request: async function() { + return { + runtimes: { + nodejs: [{ + kind: "nodejs", + image: "openwhisk/action-nodejs-v12:latest" + }] + } + } + } + } + } +}; + +const docker = new Docker(); + +async function isContainerRunning(id) { + const containers = await docker.listContainers(); + for (const container of containers) { + if (container.Id === id) { + return true; + } + } + return false; +} + + +describe('invoker', function() { + + it("should detect and replace an existing container", async function() { + // preparation: start first container with right fields using dockerode + const previousInvoker = new OpenWhiskInvoker(ACTION_NAME, ACTION_METADATA, {}, WSK_PROPS, WSK); + await previousInvoker.checkIfDockerAvailable(); + await previousInvoker.prepare(); + await previousInvoker.startContainer(() => {}); + const previousContainerId = previousInvoker.container.id; + + // start second container + const invoker = new OpenWhiskInvoker(ACTION_NAME, ACTION_METADATA, {}, WSK_PROPS, WSK); + + let id; + + try { + await invoker.prepare(); + await invoker.startContainer(() => {}); + + id = invoker.container.id; + + // verify it replaced the container (old id gone, new id with same label there) + assert.ok(!await isContainerRunning(previousContainerId), "container was not replaced"); + + } finally { + await invoker.stop(); + + if (id) { + // verify the new container is gone + assert.ok(!await isContainerRunning(id), "container was not removed"); + } + } + }).timeout(10000); +});
