Merge pull request #280 from Jigsaw-Code/fortuna-fix

Test ports by opening them

src/shadowbox/infrastructure/get_port.spec.ts

@@ -18,24 +18,21 @@ import * as get_port from './get_port';
 
 describe('PortProvider', () => {
   describe('addReservedPort', () => {
-    it('gets port over 1023', async (done) => {
+    it('gets port over 1023', async () => {
       expect(await new get_port.PortProvider().reserveNewPort()).toBeGreaterThan(1023);
-      done();
     });
-    it('fails on double reservation', (done) => {
+    it('fails on double reservation', () => {
       const ports = new get_port.PortProvider();
       ports.addReservedPort(8080);
       expect(() => ports.addReservedPort(8080)).toThrowError();
-      done();
     });
   });
   describe('freePort', () => {
-    it('makes port available', (done) => {
+    it('makes port available', () => {
       const ports = new get_port.PortProvider();
       ports.addReservedPort(8080);
       ports.freePort(8080);
       ports.addReservedPort(8080);
-      done();
     });
   });
   describe('reserverFirstFreePort', () => {
@@ -58,7 +55,7 @@ describe('PortProvider', () => {
 function listen(): Promise<net.Server> {
   const server = net.createServer();
   return new Promise((resolve, reject) => {
-    server.listen(() => {
+    server.listen({host: 'localhost', port: 0, exclusive: true}, () => {
       resolve(server);
     });
   });

src/shadowbox/infrastructure/get_port.ts

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import * as child_process from 'child_process';
+import * as net from 'net';
 
 const MAX_PORT = 65535;
 const MIN_PORT = 1024;
@@ -31,9 +32,8 @@ export class PortProvider {
 
   // Returns the first free port equal or after initialPort
   async reserveFirstFreePort(initialPort: number): Promise<number> {
-    const usedPorts = await getUsedPorts();
     for (let port = initialPort; port < 65536; port++) {
-      if (!usedPorts.has(port) && !this.reservedPorts.has(port)) {
+      if (!this.reservedPorts.has(port) && !(await isPortUsed(port))) {
         this.reservedPorts.add(port);
         return port;
       }
@@ -49,7 +49,7 @@ export class PortProvider {
       if (this.reservedPorts.has(port)) {
         continue;
       }
-      if (await isPortUsedLsof(port)) {
+      if (await isPortUsed(port)) {
         continue;
       }
       this.reservedPorts.add(port);
@@ -66,17 +66,6 @@ function getRandomPortOver1023() {
   return Math.floor(Math.random() * (MAX_PORT + 1 - MIN_PORT) + MIN_PORT);
 }
 
-// Returns the first free port equal or after initialPort
-async function getFirstFreePort(initialPort: number): Promise<number> {
-  const usedPorts = await getUsedPorts();
-  for (let port = initialPort; port < 65536; port++) {
-    if (!usedPorts.has(port)) {
-      return port;
-    }
-  }
-  throw new Error('port not found');
-}
-
 // Returns the list of ports used by either TCP or UDP.
 export function getUsedPorts(): Promise<Set<number>> {
   return new Promise((resolve, reject) => {
@@ -107,13 +96,28 @@ export function getUsedPorts(): Promise<Set<number>> {
   });
 }
 
-function isPortUsedLsof(port: number): Promise<boolean> {
-  return getUsedPorts().then((usedPorts) => {
-    for (const usedPort of usedPorts) {
-      if (usedPort === port) {
-        return true;
+interface ServerError extends Error {
+  code: string;
+}
+
+function isPortUsed(port: number): Promise<boolean> {
+  return new Promise((resolve, reject) => {
+    let isUsed = false;
+    const server = new net.Server();
+    server.on('error', (error: ServerError) => {
+      if (error.code === 'EADDRINUSE') {
+        isUsed = true;
+      } else {
+        reject(error);
       }
-    }
-    return false;
+      server.close();
+    });
+    server.listen({host: 'localhost', port, exclusive: true}, () => {
+      isUsed = false;
+      server.close();
+    });
+    server.on('close', () => {
+      resolve(isUsed);
+    });
   });
 }

src/shadowbox/server/main.ts

@@ -139,8 +139,6 @@ async function main() {
 
   logging.info('Starting...');
 
-  const legacyManagerMetrics =
-      createLegacyManagerMetrics(getPersistentFilename('shadowbox_stats.json'));
   const prometheusPort = await portProvider.reserveFirstFreePort(9090);
   const prometheusLocation = `localhost:${prometheusPort}`;
 
@@ -149,34 +147,26 @@ async function main() {
   const nodeMetricsLocation = `localhost:${nodeMetricsPort}`;
 
   const ssMetricsPort = await portProvider.reserveFirstFreePort(nodeMetricsPort + 1);
-  const ssMetricsLocation = `localhost:${ssMetricsPort}`;
   logging.info(`Prometheus is at ${prometheusLocation}`);
   logging.info(`Node metrics is at ${nodeMetricsLocation}`);
-  logging.info(`outline-ss-server metrics is at ${ssMetricsLocation}`);
 
-  runPrometheusScraper(
-      [
-        '--storage.tsdb.retention', '31d', '--storage.tsdb.path',
-        getPersistentFilename('prometheus/data'), '--web.listen-address', prometheusLocation,
-        '--log.level', verbose ? 'debug' : 'info'
-      ],
-      getPersistentFilename('prometheus/config.yml'), {
-        global: {
-          scrape_interval: '15s',
-        },
-        scrape_configs: [
-          {job_name: 'prometheus', static_configs: [{targets: [prometheusLocation]}]},
-          {job_name: 'outline-server-main', static_configs: [{targets: [nodeMetricsLocation]}]},
-          {job_name: 'outline-server-ss', static_configs: [{targets: [ssMetricsLocation]}]}
-        ]
-      });
-  const prometheusClient = new PrometheusClient(`http://${prometheusLocation}`);
-  const managerMetrics = new PrometheusManagerMetrics(prometheusClient, legacyManagerMetrics);
-  const metricsReader = new PrometheusUsageMetrics(prometheusClient);
+  const prometheusConfigJson = {
+    global: {
+      scrape_interval: '15s',
+    },
+    scrape_configs: [
+      {job_name: 'prometheus', static_configs: [{targets: [prometheusLocation]}]},
+      {job_name: 'outline-server-main', static_configs: [{targets: [nodeMetricsLocation]}]},
+    ]
+  };
 
   const rollouts = createRolloutTracker(serverConfig);
   let shadowsocksServer: ShadowsocksServer;
   if (rollouts.isRolloutEnabled('outline-ss-server', 0)) {
+    const ssMetricsLocation = `localhost:${ssMetricsPort}`;
+    logging.info(`outline-ss-server metrics is at ${ssMetricsLocation}`);
+    prometheusConfigJson.scrape_configs.push(
+        {job_name: 'outline-server-ss', static_configs: [{targets: [ssMetricsLocation]}]});
     shadowsocksServer =
         new OutlineShadowsocksServer(
             getPersistentFilename('outline-ss-server/config.yml'), verbose, ssMetricsLocation)
@@ -187,13 +177,25 @@ async function main() {
     shadowsocksServer = await createLibevShadowsocksServer(
         proxyHostname, await portProvider.reserveNewPort(), ipLocation, metricsWriter, verbose);
   }
+  runPrometheusScraper(
+      [
+        '--storage.tsdb.retention', '31d', '--storage.tsdb.path',
+        getPersistentFilename('prometheus/data'), '--web.listen-address', prometheusLocation,
+        '--log.level', verbose ? 'debug' : 'info'
+      ],
+      getPersistentFilename('prometheus/config.yml'), prometheusConfigJson);
 
   const accessKeyRepository = new ServerAccessKeyRepository(
       portProvider, proxyHostname, accessKeyConfig, shadowsocksServer);
 
+  const prometheusClient = new PrometheusClient(`http://${prometheusLocation}`);
+  const metricsReader = new PrometheusUsageMetrics(prometheusClient);
   const toMetricsId = (id: AccessKeyId) => {
     return accessKeyRepository.getMetricsId(id);
   };
+  const legacyManagerMetrics =
+      createLegacyManagerMetrics(getPersistentFilename('shadowbox_stats.json'));
+  const managerMetrics = new PrometheusManagerMetrics(prometheusClient, legacyManagerMetrics);
   const metricsCollector = new RestMetricsCollectorClient(metricsCollectorUrl);
   const metricsPublisher: SharedMetricsPublisher = new OutlineSharedMetricsPublisher(
       new RealClock(), serverConfig, metricsReader, toMetricsId, metricsCollector);

src/shadowbox/server/manager_metrics.ts

@@ -40,7 +40,7 @@ export class PrometheusManagerMetrics implements ManagerMetrics {
     }
     // TODO: Remove this after 30 days of everyone being migrated, since we won't need the config
     // file anymore.
-    this.addLegacyUsageData(usage);
+    await this.addLegacyUsageData(usage);
     return {bytesTransferredByUserId: usage};
   }
 
@@ -48,7 +48,11 @@ export class PrometheusManagerMetrics implements ManagerMetrics {
     const bytesTransferredByUserId =
         (await this.legacyManagerMetrics.get30DayByteTransfer()).bytesTransferredByUserId;
     for (const userId of Object.keys(bytesTransferredByUserId)) {
-      usage[userId] += bytesTransferredByUserId[userId];
+      const oldBytes = usage[userId] || 0;
+      const increment = bytesTransferredByUserId[userId];
+      if (increment > 0) {
+        usage[userId] = oldBytes + increment;
+      }
     }
   }
 }