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

Connect My Computer: Join cluster #29479

Merged
merged 32 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5376e1a
Add `generateAgentConfigPaths` function that creates config path base…
gzdunek Jul 24, 2023
ca6a5b2
Add functions to run agent and subscribe to its events
gzdunek Jul 24, 2023
da19a68
Clear attempts when restarting the process
gzdunek Jul 24, 2023
ea2c34f
Run the agent from the UI and remove node token
gzdunek Jul 24, 2023
8da756d
Show errors from the process in the setup UI
gzdunek Jul 24, 2023
904e253
Refactor reporting errors from the agent process
gzdunek Jul 25, 2023
ff265fa
Add `isLocalBuild`
gzdunek Jul 25, 2023
7ba35c4
Join arguments with space when logging
gzdunek Jul 25, 2023
40a6231
Add `killProcess` function that handles process closing
gzdunek Jul 26, 2023
1e3a5bd
Spawn a real process in `agentRunner` tests
gzdunek Jul 26, 2023
b32f405
Keep `agentRunner` files in a single directory
gzdunek Jul 26, 2023
b94bd7e
Catch errors from `deleteToken`
gzdunek Jul 26, 2023
72eaf48
Remove `env: process.env`
gzdunek Jul 26, 2023
50b01ef
Match on "access denied" when checking error from `deleteToken`
gzdunek Jul 28, 2023
a1ca16f
Reject when an agent process fails to start in test
gzdunek Jul 28, 2023
cdc0e8f
Match only on "ENOENT"
gzdunek Jul 28, 2023
00234d7
Correct test name ("SIGTERM" -> "SIGKILL")
gzdunek Jul 28, 2023
ec73591
Test terminating the process and then trying to kill it
gzdunek Jul 28, 2023
0a6a5e6
Wait for "exit" event instead of "close"
gzdunek Jul 28, 2023
103483b
Rename `killProcess` to `terminateWithTimeout`
gzdunek Jul 28, 2023
53e2cc5
Add `getAgentState` method to synchronously get the agent state
gzdunek Jul 31, 2023
040a50f
Remove space before new line
gzdunek Jul 31, 2023
ecdf232
Simplify the logic in `AgentRunner`
gzdunek Jul 31, 2023
086bc16
Fix TS error
gzdunek Jul 31, 2023
c80bd6e
Merge branch 'master' into gzdunek/cmc-run-agent
gzdunek Aug 1, 2023
0eebf95
Do not send agent updates to a destroyed window
gzdunek Aug 1, 2023
aef7d7e
Add logging cluster URI and updated state
gzdunek Aug 1, 2023
d3b858a
Catch errors that are thrown while spawning the process
gzdunek Aug 2, 2023
a57477a
Strip ANSI codes
gzdunek Aug 2, 2023
d80094d
Add `exitedSuccessfully` property to `exited` state, so we won't have…
gzdunek Aug 2, 2023
0b55dfd
Move `strip-ansi-stream` to `dependencies`
gzdunek Aug 2, 2023
5a466e2
Fix license
gzdunek Aug 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ test('status updates are sent on a successful start', async () => {
);

try {
expect(agentRunner.getState(rootClusterUri)).toBeUndefined();
const agentProcess = await agentRunner.start(rootClusterUri);
expect(agentRunner.getState(rootClusterUri)).toStrictEqual({
status: 'not-started',
} as AgentProcessState);
await new Promise((resolve, reject) => {
const timeout = setTimeout(
() => reject('Process start timed out.'),
Expand All @@ -95,16 +99,18 @@ test('status updates are sent on a successful start', async () => {
clearTimeout(timeout);
});
});
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
status: 'running',
} as AgentProcessState);
const runningState: AgentProcessState = { status: 'running' };
expect(agentRunner.getState(rootClusterUri)).toStrictEqual(runningState);
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, runningState);

await agentRunner.kill(rootClusterUri);
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
const exitedState: AgentProcessState = {
status: 'exited',
code: null,
signal: 'SIGTERM',
} as AgentProcessState);
};
expect(agentRunner.getState(rootClusterUri)).toBeUndefined(); // the agent has been killed and removed
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, exitedState);

expect(updateSender).toHaveBeenCalledTimes(2);
} finally {
Expand All @@ -131,10 +137,12 @@ test('status updates are sent on a failed start', async () => {
await new Promise(resolve => agentProcess.on('error', resolve));

expect(updateSender).toHaveBeenCalledTimes(1);
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, {
const errorState: AgentProcessState = {
status: 'error',
message: expect.stringContaining('ENOENT'),
} as AgentProcessState);
};
expect(agentRunner.getState(rootClusterUri)).toStrictEqual(errorState);
expect(updateSender).toHaveBeenCalledWith(rootClusterUri, errorState);
} finally {
await agentRunner.killAll();
}
Expand Down
52 changes: 42 additions & 10 deletions web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts
ravicious marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ const MAX_STDERR_LINES = 10;

export class AgentRunner {
private logger = new Logger('AgentRunner');
private agentProcesses = new Map<RootClusterUri, ChildProcess>();
private agentProcesses = new Map<
RootClusterUri,
{
process: ChildProcess;
state: AgentProcessState;
}
>();

constructor(
private settings: RuntimeSettings,
Expand All @@ -45,7 +51,6 @@ export class AgentRunner {
async start(rootClusterUri: RootClusterUri): Promise<ChildProcess> {
if (this.agentProcesses.has(rootClusterUri)) {
await this.kill(rootClusterUri);
this.logger.warn(`Killed agent process for ${rootClusterUri}`);
}

const { agentBinaryPath } = this.settings;
Expand All @@ -61,29 +66,45 @@ export class AgentRunner {
].filter(Boolean);

this.logger.info(
`Starting agent from ${agentBinaryPath} with arguments ${args.join(' ')}`
`Starting agent for ${rootClusterUri} from ${agentBinaryPath} with arguments ${args.join(
' '
)}`
);

const agentProcess = spawn(agentBinaryPath, args, {
windowsHide: true,
});

this.agentProcesses.set(rootClusterUri, {
process: agentProcess,
state: { status: 'not-started' },
});
this.addListeners(rootClusterUri, agentProcess);
this.agentProcesses.set(rootClusterUri, agentProcess);

return agentProcess;
}

getState(rootClusterUri: RootClusterUri): AgentProcessState | undefined {
return this.agentProcesses.get(rootClusterUri)?.state;
}

async kill(rootClusterUri: RootClusterUri): Promise<void> {
await terminateWithTimeout(this.agentProcesses.get(rootClusterUri));
this.agentProcesses.delete(rootClusterUri);
const agent = this.agentProcesses.get(rootClusterUri);
if (agent) {
gzdunek marked this conversation as resolved.
Show resolved Hide resolved
await terminateWithTimeout(
this.agentProcesses.get(rootClusterUri).process
);
this.agentProcesses.delete(rootClusterUri);
this.logger.info(`Killed agent for ${rootClusterUri}`);
}
this.logger.warn(`Cannot get an agent to kill for ${rootClusterUri}`);
}

async killAll(): Promise<void> {
const processes = Array.from(this.agentProcesses.entries());
await Promise.all(
processes.map(async ([rootClusterUri, agent]) => {
await terminateWithTimeout(agent);
await terminateWithTimeout(agent.process);
this.agentProcesses.delete(rootClusterUri);
})
);
Expand All @@ -102,15 +123,15 @@ export class AgentRunner {
});

const spawnHandler = () => {
this.sendProcessState(rootClusterUri, {
this.updateProcessState(rootClusterUri, {
status: 'running',
});
};

const errorHandler = (error: Error) => {
process.off('spawn', spawnHandler);

this.sendProcessState(rootClusterUri, {
this.updateProcessState(rootClusterUri, {
status: 'error',
message: `${error}`,
});
Expand All @@ -124,7 +145,7 @@ export class AgentRunner {
process.off('error', errorHandler);
process.off('spawn', spawnHandler);

this.sendProcessState(rootClusterUri, {
this.updateProcessState(rootClusterUri, {
status: 'exited',
code,
signal,
Expand All @@ -136,6 +157,17 @@ export class AgentRunner {
process.once('error', errorHandler);
process.once('exit', exitHandler);
}

private updateProcessState(
ravicious marked this conversation as resolved.
Show resolved Hide resolved
rootClusterUri: RootClusterUri,
state: AgentProcessState
): void {
const agent = this.agentProcesses.get(rootClusterUri);
if (agent) {
agent.state = state;
}
gzdunek marked this conversation as resolved.
Show resolved Hide resolved
this.sendProcessState(rootClusterUri, state);
}
}

function limitProcessOutputLines(output: string): string {
Expand Down
5 changes: 5 additions & 0 deletions web/packages/teleterm/src/mainProcess/fixtures/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { createMockFileStorage } from 'teleterm/services/fileStorage/fixtures/mo
// teleterm/services/config/index.ts reexports the config service client which depends on electron.
// Importing electron breaks the fixtures if that's done from within storybook.
import { createConfigService } from 'teleterm/services/config/configService';
import { AgentProcessState } from 'teleterm/mainProcess/types';

export class MockMainProcessClient implements MainProcessClient {
configService: ReturnType<typeof createConfigService>;
Expand Down Expand Up @@ -95,6 +96,10 @@ export class MockMainProcessClient implements MainProcessClient {
return Promise.resolve();
}

getAgentState(): AgentProcessState {
return { status: 'not-started' };
}

subscribeToAgentUpdate() {
return { cleanup: () => undefined };
}
Expand Down
12 changes: 12 additions & 0 deletions web/packages/teleterm/src/mainProcess/mainProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,18 @@ export default class MainProcess {
}
);

ipcMain.on(
'main-process-connect-my-computer-get-agent-state',
(
event,
args: {
rootClusterUri: RootClusterUri;
}
) => {
event.returnValue = this.agentRunner.getState(args.rootClusterUri);
}
);

subscribeToTerminalContextMenuEvent();
subscribeToTabContextMenuEvent();
subscribeToConfigServiceEvents(this.configService);
Expand Down
6 changes: 6 additions & 0 deletions web/packages/teleterm/src/mainProcess/mainProcessClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ export default function createMainProcessClient(): MainProcessClient {
clusterProperties
);
},
getAgentState(clusterProperties: { rootClusterUri: RootClusterUri }) {
return ipcRenderer.sendSync(
'main-process-connect-my-computer-get-agent-state',
clusterProperties
);
},
subscribeToAgentUpdate: (rootClusterUri, listener) => {
const onChange = (
_,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,14 @@ export const ConnectMyComputerContextProvider: FC<{
rootClusterUri: RootClusterUri;
}> = props => {
const { mainProcessClient, connectMyComputerService } = useAppContext();
const [agentState, setAgentState] = useState<AgentProcessState>(() => ({
status: 'not-started',
}));
const [agentState, setAgentState] = useState<AgentProcessState>(
() =>
mainProcessClient.getAgentState({
rootClusterUri: props.rootClusterUri,
}) || {
status: 'not-started',
}
);

const runAgentAndWaitForNodeToJoin = useCallback(async () => {
await connectMyComputerService.runAgent(props.rootClusterUri);
Expand Down