Merge pull request #63 from HubSpot/port-manager-tests

Port manager tests
HubSpot · Nov 14, 2023 · 8d1515f · 8d1515f
2 parents 7faaeb2 + 442b1d9
commit 8d1515f
Show file tree

Hide file tree

Showing 4 changed files with 210 additions and 26 deletions.
diff --git a/lib/__tests__/portManager.ts b/lib/__tests__/portManager.ts
@@ -0,0 +1,171 @@
+import axios from 'axios';
+import { MAX_PORT_NUMBER } from '../../constants/ports';
+import PortManagerServer from '../../utils/PortManagerServer';
+import {
+  deleteServerInstance,
+  portManagerHasActiveServers,
+  requestPorts,
+  startPortManagerServer,
+  stopPortManagerServer,
+  BASE_URL,
+} from '../portManager';
+
+const INSTANCE_ID_1 = 'test1';
+const INSTANCE_ID_2 = 'test2';
+const INSTANCE_ID_3 = 'test3';
+const INSTANCE_ID_4 = 'test4';
+
+const PORT_1 = 2345;
+const PORT_2 = 5678;
+
+const EMPTY_PORT_DATA = [
+  { instanceId: INSTANCE_ID_1 },
+  { instanceId: INSTANCE_ID_2 },
+];
+
+const PORT_DATA = [
+  { instanceId: INSTANCE_ID_1, port: PORT_1 },
+  { instanceId: INSTANCE_ID_2, port: PORT_2 },
+];
+
+const DUPLICATE_PORT_DATA = [
+  { instanceId: INSTANCE_ID_3, port: PORT_1 },
+  { instanceId: INSTANCE_ID_4, port: PORT_2 },
+];
+
+const BAD_PORT_DATA = [
+  { instanceId: INSTANCE_ID_1, port: PORT_1 },
+  { instanceId: INSTANCE_ID_2, port: MAX_PORT_NUMBER + 1 },
+];
+
+describe('lib/portManager', () => {
+  describe('startPortManagerServer()', () => {
+    it('starts the PortManagerServer', async () => {
+      expect(PortManagerServer.server).toBeUndefined();
+      await startPortManagerServer();
+      expect(PortManagerServer.server).toBeDefined();
+      await stopPortManagerServer();
+    });
+
+    it('does not fail if the PortManagerServer is already running', async () => {
+      await startPortManagerServer();
+      await startPortManagerServer();
+      expect(PortManagerServer.server).toBeDefined();
+      await stopPortManagerServer();
+    });
+  });
+
+  describe('stopPortManagerServer()', () => {
+    it('stops the PortManagerServer', async () => {
+      await startPortManagerServer();
+      expect(PortManagerServer.server).toBeDefined();
+      await stopPortManagerServer();
+      expect(PortManagerServer.server).toBeUndefined();
+    });
+
+    it('does not fail if the PortManagerServer is not running', async () => {
+      await stopPortManagerServer();
+      expect(PortManagerServer.server).toBeUndefined();
+      await stopPortManagerServer();
+    });
+  });
+
+  describe('requestPorts()', () => {
+    beforeEach(async () => {
+      await startPortManagerServer();
+    });
+    afterEach(async () => {
+      await stopPortManagerServer();
+    });
+    it('returns ports when none are specified', async () => {
+      const portData = await requestPorts(EMPTY_PORT_DATA);
+      expect(typeof portData[INSTANCE_ID_1]).toBe('number');
+      expect(typeof portData[INSTANCE_ID_2]).toBe('number');
+    });
+
+    it('returns the specified ports if not in use', async () => {
+      const portData = await requestPorts(PORT_DATA);
+      expect(portData[INSTANCE_ID_1]).toEqual(PORT_1);
+      expect(portData[INSTANCE_ID_2]).toEqual(PORT_2);
+    });
+
+    it('returns different ports if the specified ports are in use', async () => {
+      await requestPorts(PORT_DATA);
+      const portData = await requestPorts(DUPLICATE_PORT_DATA);
+      expect(portData[INSTANCE_ID_3]).not.toEqual(PORT_1);
+      expect(portData[INSTANCE_ID_4]).not.toEqual(PORT_2);
+      expect(typeof portData[INSTANCE_ID_3]).toBe('number');
+      expect(typeof portData[INSTANCE_ID_4]).toBe('number');
+    });
+
+    it('throws an error if requesting a port for a server instance that already has a port', async () => {
+      await requestPorts(PORT_DATA);
+      await expect(requestPorts(PORT_DATA)).rejects.toThrow();
+    });
+
+    it('throws an error when an invalid port is requested', async () => {
+      await expect(requestPorts(BAD_PORT_DATA)).rejects.toThrow();
+    });
+  });
+
+  describe('deleteServerInstance()', () => {
+    beforeEach(async () => {
+      await startPortManagerServer();
+    });
+    afterEach(async () => {
+      await stopPortManagerServer();
+    });
+
+    it('deletes port data for a server instance', async () => {
+      await requestPorts(PORT_DATA);
+      expect(PortManagerServer.serverPortMap[INSTANCE_ID_1]).toBeDefined();
+      await deleteServerInstance(INSTANCE_ID_1);
+      expect(PortManagerServer.serverPortMap[INSTANCE_ID_1]).toBeUndefined();
+    });
+
+    it('throws an error when attempting to delete a server instance that does not have a port', async () => {
+      await expect(deleteServerInstance(INSTANCE_ID_1)).rejects.toThrow();
+    });
+  });
+
+  describe('portManagerHasActiveServers()', () => {
+    beforeEach(async () => {
+      await startPortManagerServer();
+    });
+    afterEach(async () => {
+      await stopPortManagerServer();
+    });
+
+    it('returns false when no servers are active', async () => {
+      const hasActiveServers = await portManagerHasActiveServers();
+      expect(hasActiveServers).toBe(false);
+    });
+
+    it('returns true when servers are active', async () => {
+      await requestPorts(PORT_DATA);
+      const hasActiveServers = await portManagerHasActiveServers();
+      expect(hasActiveServers).toBe(true);
+    });
+  });
+
+  describe('PortManagerServer:getServerPortByInstanceId', () => {
+    beforeEach(async () => {
+      await startPortManagerServer();
+    });
+    afterEach(async () => {
+      await stopPortManagerServer();
+    });
+
+    it('returns the port for known server instances', async () => {
+      await requestPorts(PORT_DATA);
+      const { data } = await axios.get(`${BASE_URL}/servers/${INSTANCE_ID_1}`);
+      expect(typeof data.port).toBe('number');
+    });
+
+    it('throws an error for unknown server instances', async () => {
+      await expect(
+        axios.get(`${BASE_URL}/servers/${INSTANCE_ID_1}`)
+      ).rejects.toThrow();
+    });
+  });
+});
diff --git a/lib/portManager.ts b/lib/portManager.ts
@@ -4,7 +4,7 @@ import PortManagerServer from '../utils/PortManagerServer';
 import { detectPort } from '../utils/detectPort';
 import { PORT_MANAGER_SERVER_PORT } from '../constants/ports';
 
-const BASE_URL = `http://localhost:${PORT_MANAGER_SERVER_PORT}`;
+export const BASE_URL = `http://localhost:${PORT_MANAGER_SERVER_PORT}`;
 
 async function isPortManagerServerRunning(): Promise<boolean> {
   const port = await detectPort(PORT_MANAGER_SERVER_PORT);
@@ -45,18 +45,10 @@ export async function requestPorts(
 export async function deleteServerInstance(
   serverInstanceId: string
 ): Promise<void> {
-  await axios.post(`${BASE_URL}/servers/${serverInstanceId}`);
+  await axios.delete(`${BASE_URL}/servers/${serverInstanceId}`);
 }
 
-export async function portManagerHasActiveServers() {
+export async function portManagerHasActiveServers(): Promise<boolean> {
   const { data } = await axios.get(`${BASE_URL}/servers`);
   return data.count > 0;
 }
-
-function toId(str: string) {
-  return str.replace(/\s+/g, '-').toLowerCase();
-}
-
-export function getServerInstanceId(serverId: string, resourceId: string) {
-  return `${toId(serverId)}__${toId(resourceId)}`;
-}
diff --git a/utils/PortManagerServer.ts b/utils/PortManagerServer.ts
@@ -55,6 +55,12 @@ class PortManagerServer {
     }
   }
 
+  reset() {
+    this.app = undefined;
+    this.server = undefined;
+    this.serverPortMap = {};
+  }
+
   listen(): Promise<Server> {
     return new Promise<Server>((resolve, reject) => {
       const server = this.app!.listen(PORT_MANAGER_SERVER_PORT, () => {
@@ -125,8 +131,8 @@ class PortManagerServer {
 
     const portPromises: Array<Promise<{ [instanceId: string]: number }>> = [];
 
-    portData.forEach(data => {
-      const { port, instanceId } = data;
+    for (let i = 0; i < portData.length; i++) {
+      const { port, instanceId } = portData[i];
       if (this.serverPortMap[instanceId]) {
         res.status(409).send(
           i18n(`${i18nKey}.errors.409`, {
@@ -146,16 +152,18 @@ class PortManagerServer {
       } else {
         const promise = new Promise<{ [instanceId: string]: number }>(
           resolve => {
-            detectPort(port).then(resolvedPort => {
-              resolve({
-                [instanceId]: resolvedPort,
-              });
-            });
+            detectPort(port, Object.values(this.serverPortMap)).then(
+              resolvedPort => {
+                resolve({
+                  [instanceId]: resolvedPort,
+                });
+              }
+            );
           }
         );
         portPromises.push(promise);
       }
-    });
+    }
 
     const portList = await Promise.all(portPromises);
     const ports = portList.reduce((a, c) => Object.assign(a, c));
@@ -184,6 +192,7 @@ class PortManagerServer {
       debug(`${i18nKey}.close`);
       res.send(200);
       this.server.close();
+      this.reset();
     }
   };
 }

diff --git a/utils/detectPort.ts b/utils/detectPort.ts
@@ -37,7 +37,10 @@ type ListenCallback = (error: NetError | null, port: number) => void;
 
 const i18nKey = 'utils.detectPort';
 
-export function detectPort(port?: number | null): Promise<number> {
+export function detectPort(
+  port?: number | null,
+  exclude: Array<number> = []
+): Promise<number> {
   if (port && (port < MIN_PORT_NUMBER || port > MAX_PORT_NUMBER)) {
     throwErrorWithMessage(`${i18nKey}.errors.invalidPort`, {
       minPort: MIN_PORT_NUMBER,
@@ -49,13 +52,18 @@ export function detectPort(port?: number | null): Promise<number> {
   const maxPort = Math.min(portToUse + 10, MAX_PORT_NUMBER);
 
   return new Promise(resolve => {
-    tryListen(portToUse, maxPort, (_, resolvedPort) => {
+    tryListen(portToUse, maxPort, exclude, (_, resolvedPort) => {
       resolve(resolvedPort);
     });
   });
 }
 
-function tryListen(port: number, maxPort: number, callback: ListenCallback) {
+function tryListen(
+  port: number,
+  maxPort: number,
+  exclude: Array<number>,
+  callback: ListenCallback
+) {
   const shouldGiveUp = port >= maxPort;
   const nextPort = shouldGiveUp ? 0 : port + 1;
   const nextMaxPort = shouldGiveUp ? 0 : maxPort;
@@ -66,26 +74,30 @@ function tryListen(port: number, maxPort: number, callback: ListenCallback) {
       return callback(err, realPort);
     }
 
+    if (exclude.includes(port)) {
+      return tryListen(nextPort, nextMaxPort, exclude, callback);
+    }
+
     if (err) {
-      return tryListen(nextPort, nextMaxPort, callback);
+      return tryListen(nextPort, nextMaxPort, exclude, callback);
     }
 
     // 2. check 0.0.0.0
     listen(port, '0.0.0.0', err => {
       if (err) {
-        return tryListen(nextPort, nextMaxPort, callback);
+        return tryListen(nextPort, nextMaxPort, exclude, callback);
       }
 
       // 3. check localhost
       listen(port, 'localhost', err => {
         if (err && err.code !== 'EADDRNOTAVAIL') {
-          return tryListen(nextPort, nextMaxPort, callback);
+          return tryListen(nextPort, nextMaxPort, exclude, callback);
         }
 
         // 4. check current ip
         listen(port, ip(), (err, realPort) => {
           if (err) {
-            return tryListen(nextPort, nextMaxPort, callback);
+            return tryListen(nextPort, nextMaxPort, exclude, callback);
           }
 
           callback(null, realPort);