You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by al...@apache.org on 2020/07/17 05:01:24 UTC

[openwhisk-wskdebug] 01/01: detect if debug port is already used and remove left over container #59

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 <ak...@adobe.com>
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);
+});