-
Notifications
You must be signed in to change notification settings - Fork 3
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
Port manager #42
Port manager #42
Changes from 19 commits
bcd10b7
d028563
82cd169
ecc19c8
9b660ea
e151907
75726b0
5766ea7
c2e3013
060b9f6
d5ce5cd
61d81d4
b83750f
73d05b1
90ae978
e79df2a
1d30147
91a3228
0bb5532
d191ff3
053dba6
872304f
2726eec
de09bad
25eb5ea
825d86b
bd1e9cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export const MIN_PORT_NUMBER = 1024; | ||
export const MAX_PORT_NUMBER = 65535; | ||
|
||
// TODO: What's a good port for the port manager? | ||
export const PORT_MANAGER_SERVER_PORT = 8080; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,6 +136,12 @@ en: | |
processFieldsJs: | ||
converting: 'Converting "{{ src }}" to "{{ dest }}".' | ||
converted: 'Finished converting "{{ src }}" to "{{ dest }}".' | ||
utils: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm realizing we're going to hit some merge conflicts with all of the WIP PR's and my translations spike PR |
||
PortManagerServer: | ||
started: "PortManagerServer running on port {{ port }}" | ||
setPort: "Server with instanceId {{ instanceId }} assigned to port {{ port }}" | ||
deletedPort: "Server with instanceId {{ instanceId }} unassigned from port {{ port }}" | ||
close: "PortManagerServer shutting down." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should refer to this differently in logs. End users probably don't need to know about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's true, though these are all debug logs so theoretically end users wouldn't be seeing them. Seemed helpful if anyone was ever debugging something related to this or the inner workings became relevant to their workflow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok, I was thinking that they were standard logs. That makes sense 👌 |
||
errors: | ||
hubdb: | ||
invalidJsonPath: "The HubDB table file must be a '.json' file" | ||
|
@@ -201,6 +207,12 @@ en: | |
configIgnore: "Unable to determine if config file is properly ignored by git." | ||
notify: | ||
filePath: "Unable to notify file '{{ filePath }}'" | ||
PortManagerServer: | ||
portInUse: "Failed to start PortManagerServer. Port {{ port }} is already in use." | ||
duplicateInstance: "Failed to start PortManagerServer. An instance of PortManagerServer is already running." | ||
404: "Could not find a server with instanceId {{ instanceId }}" | ||
409: "Failed to assign port. Server with instanceId {{ instanceId }} is already running on port {{ port }}" | ||
400: "Invalid port requested. Port must be between 1024 and 65535." | ||
config: | ||
cliConfiguration: | ||
noConfigLoaded: "No config loaded." | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import axios from 'axios'; | ||
|
||
import PortManagerServer from '../utils/PortManagerServer'; | ||
import { detectPort } from '../utils/detectPort'; | ||
import { PORT_MANAGER_SERVER_PORT } from '../constants/ports'; | ||
import { RequestPortsData } from '../types/PortManager'; | ||
|
||
const BASE_URL = `http://localhost:${PORT_MANAGER_SERVER_PORT}`; | ||
|
||
async function isPortManagerServerRunning(): Promise<boolean> { | ||
const port = await detectPort(PORT_MANAGER_SERVER_PORT); | ||
return port !== PORT_MANAGER_SERVER_PORT; | ||
} | ||
|
||
export async function startPortManagerServer(): Promise<void> { | ||
const isRunning = await isPortManagerServerRunning(); | ||
|
||
if (!isRunning) { | ||
await PortManagerServer.init(); | ||
} | ||
} | ||
|
||
export async function stopPortManagerServer(): Promise<void> { | ||
const isRunning = await isPortManagerServerRunning(); | ||
|
||
if (isRunning) { | ||
await axios.post(`${BASE_URL}/close`); | ||
} | ||
} | ||
|
||
export async function requestPorts( | ||
portData: Array<RequestPortsData> | ||
): Promise<Array<RequestPortsData>> { | ||
const { data } = await axios.post(`${BASE_URL}/servers`, { | ||
portData: portData, | ||
}); | ||
|
||
return data.ports; | ||
} | ||
|
||
export async function deleteServerInstance( | ||
serverInstanceId: string | ||
): Promise<void> { | ||
await axios.post(`${BASE_URL}/servers/${serverInstanceId}`); | ||
} | ||
|
||
export async function portManagerHasActiveServers() { | ||
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)}`; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export type RequestPortsData = { | ||
instanceId: string; | ||
port?: number; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
import express, { Express, Request, Response } from 'express'; | ||
import { Server } from 'http'; | ||
import cors from 'cors'; | ||
|
||
import { detectPort } from './detectPort'; | ||
import { | ||
MIN_PORT_NUMBER, | ||
MAX_PORT_NUMBER, | ||
PORT_MANAGER_SERVER_PORT, | ||
} from '../constants/ports'; | ||
import { throwErrorWithMessage } from '../errors/standardErrors'; | ||
import { debug } from './logger'; | ||
import { i18n } from './lang'; | ||
import { BaseError } from '../types/Error'; | ||
import { RequestPortsData } from '../types/PortManager'; | ||
|
||
type ServerPortMap = { | ||
[instanceId: string]: number; | ||
}; | ||
|
||
const i18nKey = 'utils.PortManagerServer'; | ||
|
||
class PortManagerServer { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a blocker - I wonder if we should even bother having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah 🤷 . Was left over from cli-lib but definitely don't think we need to make the distinction. I'm down to remove in the future |
||
app?: Express; | ||
server?: Server; | ||
serverPortMap: ServerPortMap; | ||
|
||
constructor() { | ||
this.serverPortMap = {}; | ||
} | ||
|
||
async init(): Promise<void> { | ||
if (this.app) { | ||
throwErrorWithMessage(`${i18nKey}.duplicateInstance`); | ||
} | ||
this.app = express(); | ||
this.app.use(express.json()); | ||
this.app.use(cors()); | ||
this.setupRoutes(); | ||
|
||
try { | ||
this.server = await this.listen(); | ||
} catch (e) { | ||
const error = e as BaseError; | ||
if (error.code === 'EADDRINUSE') { | ||
throwErrorWithMessage( | ||
`${i18n}.portInUse`, | ||
{ | ||
port: PORT_MANAGER_SERVER_PORT, | ||
}, | ||
error | ||
); | ||
} | ||
throw error; | ||
} | ||
} | ||
|
||
listen(): Promise<Server> { | ||
return new Promise<Server>((resolve, reject) => { | ||
const server = this.app!.listen(PORT_MANAGER_SERVER_PORT, () => { | ||
debug(`${i18nKey}.started`, { | ||
port: PORT_MANAGER_SERVER_PORT, | ||
}); | ||
resolve(server); | ||
}).on('error', err => { | ||
reject(err); | ||
}); | ||
}); | ||
} | ||
|
||
setupRoutes(): void { | ||
if (!this.app) { | ||
return; | ||
} | ||
|
||
this.app.get('/servers', this.getServers); | ||
this.app.get('/servers/:instanceId', this.getServerPortByInstanceId); | ||
this.app.post('/servers', this.assignPortsToServers); | ||
this.app.delete('/servers/:instanceId', this.deleteServerInstance); | ||
this.app.post('/close', this.closeServer); | ||
} | ||
|
||
setPort(instanceId: string, port: number) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this protect against initializing duplicate instanceId's? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
debug(`${i18nKey}.setPort`, { instanceId, port }); | ||
this.serverPortMap[instanceId] = port; | ||
} | ||
|
||
deletePort(instanceId: string) { | ||
debug(`${i18nKey}.deletePort`, { | ||
instanceId, | ||
port: this.serverPortMap[instanceId], | ||
}); | ||
delete this.serverPortMap[instanceId]; | ||
} | ||
|
||
send404(res: Response, instanceId: string) { | ||
res | ||
.status(404) | ||
.send(i18n(`errors.${i18nKey}.404`, { instanceId: instanceId })); | ||
} | ||
|
||
getServers = async (req: Request, res: Response): Promise<void> => { | ||
res.send({ | ||
servers: this.serverPortMap, | ||
count: Object.keys(this.serverPortMap).length, | ||
}); | ||
}; | ||
|
||
getServerPortByInstanceId = (req: Request, res: Response): void => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the return on this be Promise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't thinks so, this isn't async. Express knows how to handle it as is |
||
const { instanceId } = req.params; | ||
const port = this.serverPortMap[instanceId]; | ||
|
||
if (port) { | ||
res.send({ port }); | ||
} else { | ||
this.send404(res, instanceId); | ||
} | ||
}; | ||
|
||
assignPortsToServers = async ( | ||
req: Request<never, never, { portData: Array<RequestPortsData> }>, | ||
res: Response | ||
): Promise<void> => { | ||
const { portData } = req.body; | ||
|
||
const portPromises: Array<Promise<Required<RequestPortsData>>> = []; | ||
|
||
portData.forEach(data => { | ||
const { port, instanceId } = data; | ||
if (this.serverPortMap[instanceId]) { | ||
res.status(409).send( | ||
i18n(`errors.${i18nKey}.409`, { | ||
instanceId, | ||
port: this.serverPortMap[instanceId], | ||
}) | ||
); | ||
return; | ||
} else if (port && (port < MIN_PORT_NUMBER || port > MAX_PORT_NUMBER)) { | ||
res.status(400).send(i18n(`errors.${i18nKey}.400`)); | ||
return; | ||
} else { | ||
const promise = new Promise<Required<RequestPortsData>>(resolve => { | ||
detectPort(port).then(resolvedPort => { | ||
resolve({ | ||
instanceId, | ||
port: resolvedPort, | ||
}); | ||
}); | ||
}); | ||
portPromises.push(promise); | ||
} | ||
}); | ||
|
||
const ports = await Promise.all(portPromises); | ||
|
||
ports.forEach(({ port, instanceId }) => { | ||
this.setPort(instanceId, port); | ||
}); | ||
|
||
res.send({ ports }); | ||
}; | ||
|
||
deleteServerInstance = (req: Request, res: Response): void => { | ||
const { instanceId } = req.params; | ||
const port = this.serverPortMap[instanceId]; | ||
|
||
if (port) { | ||
this.deletePort(instanceId); | ||
res.send(200); | ||
} else { | ||
this.send404(res, instanceId); | ||
} | ||
}; | ||
|
||
closeServer = (req: Request, res: Response): void => { | ||
if (this.server) { | ||
debug(`${i18nKey}.close`); | ||
res.send(200); | ||
this.server.close(); | ||
} | ||
}; | ||
} | ||
|
||
export default new PortManagerServer(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
import net, { AddressInfo } from 'net'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may need to include the copyright info in here per the license: https://github.com/node-modules/detect-port/blob/master/LICENSE There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. I could have sworn I had already done this |
||
import { ip } from 'address'; | ||
|
||
import { MIN_PORT_NUMBER, MAX_PORT_NUMBER } from '../constants/ports'; | ||
|
||
type NetError = Error & { | ||
code: string; | ||
}; | ||
|
||
type ListenCallback = (error: NetError | null, port: number) => void; | ||
|
||
export function detectPort(port?: number | null): Promise<number> { | ||
if (port && (port < MIN_PORT_NUMBER || port > MAX_PORT_NUMBER)) { | ||
throw new Error('Port must be between 1024 and 65535'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should put this copy in the lang file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good call, I guess I'll hold off on that until the lang stuff is merged |
||
} | ||
|
||
const portToUse = port || 0; | ||
const maxPort = Math.min(portToUse + 10, MAX_PORT_NUMBER); | ||
|
||
return new Promise(resolve => { | ||
tryListen(portToUse, maxPort, (_, resolvedPort) => { | ||
resolve(resolvedPort); | ||
}); | ||
}); | ||
} | ||
|
||
function tryListen(port: number, maxPort: number, callback: ListenCallback) { | ||
const shouldGiveUp = port >= maxPort; | ||
const nextPort = shouldGiveUp ? 0 : port + 1; | ||
const nextMaxPort = shouldGiveUp ? 0 : maxPort; | ||
|
||
listen(port, undefined, (err, realPort) => { | ||
// ignore random listening | ||
if (port === 0) { | ||
return callback(err, realPort); | ||
} | ||
|
||
if (err) { | ||
return tryListen(nextPort, nextMaxPort, callback); | ||
} | ||
|
||
// 2. check 0.0.0.0 | ||
listen(port, '0.0.0.0', err => { | ||
if (err) { | ||
return tryListen(nextPort, nextMaxPort, callback); | ||
} | ||
|
||
// 3. check localhost | ||
listen(port, 'localhost', err => { | ||
if (err && err.code !== 'EADDRNOTAVAIL') { | ||
return tryListen(nextPort, nextMaxPort, callback); | ||
} | ||
|
||
// 4. check current ip | ||
listen(port, ip(), (err, realPort) => { | ||
if (err) { | ||
return tryListen(nextPort, nextMaxPort, callback); | ||
} | ||
|
||
callback(null, realPort); | ||
}); | ||
}); | ||
}); | ||
}); | ||
} | ||
|
||
function listen( | ||
port: number, | ||
hostname: string | undefined, | ||
callback: ListenCallback | ||
): void { | ||
const server = new net.Server(); | ||
|
||
server.on('error', (err: NetError) => { | ||
server.close(); | ||
if (err.code === 'ENOTFOUND') { | ||
return callback(null, port); | ||
} | ||
return callback(err, 0); | ||
}); | ||
|
||
server.listen(port, hostname, () => { | ||
const addressInfo = server.address() as AddressInfo; | ||
server.close(); | ||
return callback(null, addressInfo.port); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lingering todo (unless you still plan to revisit this)