Connect My Computer: Join cluster

web/packages/teleterm/src/mainProcess/agentRunner.test.ts

@@ -14,9 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import childProcess, { ChildProcess } from 'node:child_process';
-import { EventEmitter } from 'node:events';
-import { PassThrough } from 'node:stream';
+import path from 'node:path';
 
 import Logger, { NullService } from 'teleterm/logger';
 import { RootClusterUri } from 'teleterm/ui/uri';
@@ -26,72 +24,56 @@ import { makeRuntimeSettings } from './fixtures/mocks';
 import { AgentRunner } from './agentRunner';
 import { AgentProcessState } from './types';
 
-jest.mock('node:child_process');
-
-let eventEmitter: EventEmitter;
-let childProcessMock: ChildProcess;
 beforeEach(() => {
   Logger.init(new NullService());
-
-  eventEmitter = new EventEmitter();
-  childProcessMock = {
-    stderr: new PassThrough(),
-    once: (event, listener) => {
-      eventEmitter.once(event, listener);
-      return this;
-    },
-    on: (event, listener) => {
-      eventEmitter.on(event, listener);
-      return this;
-    },
-    off: (event, listener) => {
-      eventEmitter.off(event, listener);
-      return this;
-    },
-    kill: jest.fn().mockImplementation(() => {
-      eventEmitter.emit('exit', 0);
-    }),
-  } as unknown as ChildProcess;
-
-  jest.spyOn(childProcess, 'spawn').mockReturnValue(childProcessMock);
 });
 
 const userDataDir = '/Users/test/Application Data/Teleport Connect';
-const agentBinaryPath = '/Users/test/Caches/Teleport Connect/teleport/teleport';
+const agentBinaryPath = path.join(__dirname, 'agentTestProcess.mjs');
 const rootClusterUri: RootClusterUri = '/clusters/cluster.local';
 
-test('agent process starts with correct arguments', () => {
+test('agent process starts with correct arguments', async () => {
   const agentRunner = new AgentRunner(
     makeRuntimeSettings({
       agentBinaryPath,
       userDataDir,
     }),
     () => {}
   );
-  agentRunner.start(rootClusterUri);
 
-  expect(childProcess.spawn).toHaveBeenCalledWith(
-    agentBinaryPath,
-    ['start', `--config=${userDataDir}/agents/cluster.local/config.yaml`],
-    expect.anything()
-  );
+  try {
+    const agentProcess = await agentRunner.start(rootClusterUri);
+
+    expect(agentProcess.spawnargs).toEqual([
+      agentBinaryPath,
+      'start',
+      `--config=${userDataDir}/agents/cluster.local/config.yaml`,
+    ]);
+  } finally {
+    await agentRunner.killAll();
+  }
 });
 
-test('previous agent process is killed when a new one is started', () => {
+test('previous agent process is killed when a new one is started', async () => {
   const agentRunner = new AgentRunner(
     makeRuntimeSettings({
       agentBinaryPath,
       userDataDir,
     }),
     () => {}
   );
-  agentRunner.start(rootClusterUri);
-  agentRunner.start(rootClusterUri);
 
-  expect(childProcessMock.kill).toHaveBeenCalledWith('SIGKILL');
+  try {
+    const firstProcess = await agentRunner.start(rootClusterUri);
+    await agentRunner.start(rootClusterUri);
+
+    expect(firstProcess.killed).toBeTruthy();
+  } finally {
+    await agentRunner.killAll();
+  }
 });
 
-test('status updates are sent', () => {
+test('status updates are sent on a successful start', async () => {
   const updateSender = jest.fn();
   const agentRunner = new AgentRunner(
     makeRuntimeSettings({
@@ -101,21 +83,47 @@ test('status updates are sent', () => {
     updateSender
   );
 
-  agentRunner.start(rootClusterUri);
-  expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
-    status: 'running',
-  } as AgentProcessState);
-
-  const error = new Error('unknown error');
-  eventEmitter.emit('error', error);
-  expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
-    status: 'error',
-    message: `${error}`,
-  } as AgentProcessState);
-
-  agentRunner.kill(rootClusterUri);
-  expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
-    status: 'exited',
-    code: 0,
-  } as AgentProcessState);
+  try {
+    const agentProcess = await agentRunner.start(rootClusterUri);
+    await new Promise(resolve => agentProcess.on('spawn', resolve));
+    expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
+      status: 'running',
+    } as AgentProcessState);
+
+    await agentRunner.kill(rootClusterUri);
+    expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
+      status: 'exited',
+      code: null,
+      signal: 'SIGTERM',
+    } as AgentProcessState);
+
+    expect(updateSender).toHaveBeenCalledTimes(2);
+  } finally {
+    await agentRunner.killAll();
+  }
+});
+
+test('status updates are sent on a failed start', async () => {
+  const updateSender = jest.fn();
+  const nonExisingPath = path.join(__dirname, 'agentTestProcess-nonExisting.mjs');
+  const agentRunner = new AgentRunner(
+    makeRuntimeSettings({
+      agentBinaryPath: nonExisingPath,
+      userDataDir,
+    }),
+    updateSender
+  );
+
+  try {
+    const agentProcess = await agentRunner.start(rootClusterUri);
+    await new Promise(resolve => agentProcess.on('error', resolve));
+
+    expect(updateSender).toHaveBeenCalledTimes(1);
+    expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
+      status: 'error',
+      message: `Error: spawn ${nonExisingPath} ENOENT`,
+    } as AgentProcessState);
+  } finally {
+    await agentRunner.killAll();
+  }
 });

web/packages/teleterm/src/mainProcess/agentRunner.ts

@@ -42,7 +42,7 @@ export class AgentRunner {
    * Starts a new agent process.
    * If an existing process exists for the given root cluster, the old one will be killed.
    */
-  async start(rootClusterUri: RootClusterUri): Promise<void> {
+  async start(rootClusterUri: RootClusterUri): Promise<ChildProcess> {
     if (this.agentProcesses.has(rootClusterUri)) {
       await this.kill(rootClusterUri);
       this.logger.warn(`Killed agent process for ${rootClusterUri}`);
@@ -69,12 +69,10 @@ export class AgentRunner {
       env: process.env,
     });
 
-    this.sendProcessState(rootClusterUri, {
-      status: 'running',
-    });
-
     this.addListeners(rootClusterUri, agentProcess);
     this.agentProcesses.set(rootClusterUri, agentProcess);
+
+    return agentProcess;
   }
 
   async kill(rootClusterUri: RootClusterUri): Promise<void> {
@@ -104,7 +102,15 @@ export class AgentRunner {
       stderrOutput = limitProcessOutputLines(stderrOutput);
     });
 
+    const spawnHandler = () => {
+      this.sendProcessState(rootClusterUri, {
+        status: 'running',
+      });
+    };
+
     const errorHandler = (error: Error) => {
+      process.off('spawn', spawnHandler);
+
       this.sendProcessState(rootClusterUri, {
         status: 'error',
         message: `${error}`,
@@ -115,17 +121,19 @@ export class AgentRunner {
       code: number | null,
       signal: NodeJS.Signals | null
     ) => {
-      // Remove error handler when the process exits.
+      // Remove handlers when the process exits.
       process.off('error', errorHandler);
+      process.off('spawn', spawnHandler);
 
       this.sendProcessState(rootClusterUri, {
         status: 'exited',
         code,
         signal,
-        stackTrace: code !== 0 ? stderrOutput : undefined,
+        stackTrace: signal !== 'SIGTERM' ? stderrOutput : undefined,
       });
     };
 
+    process.once('spawn', spawnHandler);
     process.once('error', errorHandler);
     process.once('exit', exitHandler);
   }

web/packages/teleterm/src/mainProcess/agentTestProcess.mjs

@@ -0,0 +1,21 @@
+#!/usr/bin/env node
+/**
+ * Copyright 2023 Gravitational, Inc
+ *
+ * Licensed 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.
+ */
+
+
+import { setTimeout } from 'node:timers/promises';
+
+await setTimeout(10_000);