Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker-compose port format has changed #183

Closed
n1ru4l opened this issue Sep 17, 2020 · 2 comments · Fixed by #184
Closed

docker-compose port format has changed #183

n1ru4l opened this issue Sep 17, 2020 · 2 comments · Fixed by #184

Comments

@n1ru4l
Copy link
Collaborator

n1ru4l commented Sep 17, 2020

I updated to docker desktop 2.3.0.5 and it broke running dockest and my first investigation showed that the docker-compose.yml format at runtime has changed from { target: 6379, published: 7004 } (object) to "7004:6379/tcp" (string).

I created a hotfix via patch-package.

I will hopefully soon also create a patch over here. We should definitely use some kind of decoder pattern to ensure the runtime types from IO (docker-compose) are actually correct and if not fail early and maybe hint the user to report the change here.

@erikengervall Do you have any preference for the library? I have used https://github.com/gcanti/io-ts before but there are also alternatives like https://github.com/sideway/joi or

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Sep 17, 2020

This is the patch we are using right now. Note we did not upgrade to v2 yet. However, this issue will most likely affect v2 too.

diff --git a/node_modules/dockest/dist/index.js b/node_modules/dockest/dist/index.js
index 6a76b09..373623f 100644
--- a/node_modules/dockest/dist/index.js
+++ b/node_modules/dockest/dist/index.js
@@ -30,7 +30,7 @@ class Dockest {
         };
         this.run = async () => {
             this.config.$.perfStart = Date.now();
-            this.config.$.isInsideDockerContainer = is_docker_1.default();
+            this.config.$.isInsideDockerContainer = false;
             if (this.config.$.isInsideDockerContainer) {
                 this.config.runners.forEach(runner => {
                     runner.isBridgeNetworkMode = true;
diff --git a/node_modules/dockest/dist/onRun/waitForRunnersReadiness/checkConnection.js b/node_modules/dockest/dist/onRun/waitForRunnersReadiness/checkConnection.js
index 9f79e4d..5c22b25 100644
--- a/node_modules/dockest/dist/onRun/waitForRunnersReadiness/checkConnection.js
+++ b/node_modules/dockest/dist/onRun/waitForRunnersReadiness/checkConnection.js
@@ -29,7 +29,22 @@ exports.testables = testables;
 exports.default = async (runner) => {
     const { runnerConfig: { host, service, connectionTimeout, ports }, logger, } = runner;
     const portKey = runner.isBridgeNetworkMode ? 'target' : 'published';
-    for (const { [portKey]: port } of ports) {
+    for (const record of ports) {
+        let port = null
+        if (typeof record === "object") {
+            port = record[portKey]
+        } else if (typeof record === "string") {
+            const parseRegex = /(\d*):(\d*)\/\w*/
+            const match = record.match(parseRegex);
+            if (match) {
+                port = portKey === "target" ? parseInt(match[2], 10) : parseInt(match[1], 10)
+            }
+        }
+
+        if (!port) {
+            throw new Error("docker-compose.yml is invalid. Could not resolve port.")
+        }
+
         const recurse = async (connectionTimeout) => {
             logger.debug(`Checking connection (${host}:${port}) (Timeout in: ${connectionTimeout}s)`);
             if (connectionTimeout <= 0) {
diff --git a/node_modules/dockest/dist/onRun/waitForRunnersReadiness/runRunnerCommands.js b/node_modules/dockest/dist/onRun/waitForRunnersReadiness/runRunnerCommands.js
index 329231c..3a710e4 100644
--- a/node_modules/dockest/dist/onRun/waitForRunnersReadiness/runRunnerCommands.js
+++ b/node_modules/dockest/dist/onRun/waitForRunnersReadiness/runRunnerCommands.js
@@ -6,7 +6,10 @@ Object.defineProperty(exports, "__esModule", { value: true });
 const execa_1 = __importDefault(require("execa")); // eslint-disable-line import/default
 exports.default = async (runner) => {
     const { runnerConfig: { commands = [] }, logger, } = runner;
-    for (const command of commands) {
+    for (let command of commands) {
+        if (typeof command === "function") {
+            command = command(runner.containerId)
+        }
         logger.debug(`Executed custom command: ${command}`);
         const { stdout: result } = await execa_1.default(command, { shell: true });
         logger.debug(`Executed custom command successfully with result: ${result}`, { nl: 1 });
diff --git a/node_modules/dockest/dist/runners/@types.d.ts b/node_modules/dockest/dist/runners/@types.d.ts
index 32d4ca8..33e9498 100644
--- a/node_modules/dockest/dist/runners/@types.d.ts
+++ b/node_modules/dockest/dist/runners/@types.d.ts
@@ -64,7 +64,7 @@ export declare type PortBindingType = {
 };
 export interface SharedDefaultableConfigProps {
     build: string | undefined | any;
-    commands: string[];
+    commands: (string | ((containerId: string) => string))[];
     connectionTimeout: number;
     dependsOn: DependsOn;
     host: string;
diff --git a/node_modules/dockest/dist/test-helper/index.js b/node_modules/dockest/dist/test-helper/index.js
index fe0064d..1205266 100644
--- a/node_modules/dockest/dist/test-helper/index.js
+++ b/node_modules/dockest/dist/test-helper/index.js
@@ -4,7 +4,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
 };
 Object.defineProperty(exports, "__esModule", { value: true });
 const is_docker_1 = __importDefault(require("is-docker")); // eslint-disable-line import/default
-const isInsideDockerContainer = is_docker_1.default();
+const isInsideDockerContainer = false;
 if (!process.env.DOCKEST_INTERNAL_CONFIG) {
     throw new Error('Not executed inside dockest context.');
 }
@@ -16,13 +16,36 @@ exports.getHostAddress = () => {
     return `host.dockest-runner.internal`;
 };
 exports.getServiceAddress = (serviceName, targetPort) => {
+    // console.log(JSON.stringify(config, null, 2))
     const service = config.services[serviceName];
     if (!service) {
         throw new Error(`Service "${serviceName}" does not exist.`);
     }
-    const portBinding = service.ports.find(portBinding => portBinding.target === targetPort);
+    let portBinding = null;
+    service.ports.find(record => {
+        if (typeof record === "object") {
+            if (record.target === targetPort) {
+                portBinding = record;
+                return true;
+            }
+            return false;
+        } else if (typeof record === "string") {
+            const parseRegex = /(\d*):(\d*)\/\w*/
+            const match = record.match(parseRegex);
+            if (match) {
+                const target = parseInt(match[2], 10);
+                const published = parseInt(match[1], 10);
+                console.log(target, targetPort)
+                if (target === targetPort) {
+                    portBinding = { target, published };
+                }
+                return true;
+            }
+            return false;
+        }
+    });
     if (!portBinding) {
-        throw new Error(`Service "${serviceName}" has no target port ${portBinding}.`);
+        throw new Error(`Service "${serviceName}" has no target port ${targetPort}.`);
     }
     if (isInsideDockerContainer) {
         return `${serviceName}:${portBinding.target}`;

@erikengervall
Copy link
Owner

I updated to docker desktop 2.3.0.5 and it broke running dockest and my first investigation showed that the docker-compose.yml format at runtime has changed from { target: 6379, published: 7004 } (object) to "7004:6379/tcp" (string).

I created a hotfix via patch-package.

Awesome! 👍

I will hopefully soon also create a patch over here. We should definitely use some kind of decoder pattern to ensure the runtime types from IO (docker-compose) are actually correct and if not fail early and maybe hint the user to report the change here.

@erikengervall Do you have any preference for the library? I have used https://github.com/gcanti/io-ts before but there are also alternatives like https://github.com/sideway/joi or

Have used joi previously and found it useful. Don't have any strong preferences though, whatever gets the job done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants