Skip to content

Commit

Permalink
refactor(benchmark): Use ssh instead of az ssh vm
Browse files Browse the repository at this point in the history
While the `az ssh vm` is more secure, it has multiple issues:
- The access token expires in 10 mins, causing test failures:
  `ERROR: AADSTS700024: Client assertion is not within its valid time range`
- It does not propagate the exit code, but instead always exits with 0
- It can't do `scp`, so we need to clone the repo. This makes local development more cumbersome
- It logs all kinds of unnecessary stuff into stdout

Hence this commits replaces `az ssh vm` with `ssh` and `scp`. Since the benchmark
environment is a throwaway env without any critical data, it's not the biggest deals.
We keep the private key hidden throughout the run anyways, so it's not leaked unless
someone gains access to the GH runner.
  • Loading branch information
tomi committed Sep 2, 2024
1 parent f257fd1 commit a03a797
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 23 deletions.
4 changes: 2 additions & 2 deletions packages/@n8n/benchmark/infra/benchmark-env.tf
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ resource "azurerm_dedicated_host_group" "main" {
automatic_placement_enabled = false
zone = 1

tags = local.common_tags
tags = local.common_tags
}

resource "azurerm_dedicated_host" "hosts" {
Expand All @@ -35,7 +35,7 @@ resource "azurerm_dedicated_host" "hosts" {
sku_name = var.host_size_family
platform_fault_domain = 0

tags = local.common_tags
tags = local.common_tags
}

# VM
Expand Down
4 changes: 4 additions & 0 deletions packages/@n8n/benchmark/infra/modules/benchmark-vm/output.tf
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ output "vm_name" {
output "ip" {
value = azurerm_public_ip.main.ip_address
}

output "ssh_username" {
value = azurerm_linux_virtual_machine.main.admin_username
}
6 changes: 3 additions & 3 deletions packages/@n8n/benchmark/infra/modules/benchmark-vm/vm.tf
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ resource "azurerm_linux_virtual_machine" "main" {
version = "latest"
}

identity {
type = "SystemAssigned"
}
identity {
type = "SystemAssigned"
}

tags = var.tags
}
Expand Down
13 changes: 13 additions & 0 deletions packages/@n8n/benchmark/infra/output.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
output "vm_name" {
value = module.test_vm.vm_name
}

output "ip" {
value = module.test_vm.ip
}

output "ssh_username" {
value = module.test_vm.ssh_username
}

output "ssh_private_key" {
value = tls_private_key.ssh_key.private_key_pem
sensitive = true
}
2 changes: 1 addition & 1 deletion packages/@n8n/benchmark/infra/vars.tf
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ locals {
Id = "N8nBenchmark"
Terraform = "true"
Owner = "Catalysts"
CreatedAt = timestamp()
CreatedAt = timestamp()
}
}
19 changes: 14 additions & 5 deletions packages/@n8n/benchmark/scripts/clients/sshClient.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { $ } from 'zx';
export class SshClient {
/**
*
* @param {{ vmName: string; resourceGroupName: string; verbose?: boolean }} param0
* @param {{ privateKeyPath: string; ip: string; username: string; verbose?: boolean }} param0
*/
constructor({ vmName, resourceGroupName, verbose = false }) {
this.vmName = vmName;
this.resourceGroupName = resourceGroupName;
constructor({ privateKeyPath, ip, username, verbose = false }) {
this.verbose = verbose;
this.privateKeyPath = privateKeyPath;
this.ip = ip;
this.username = username;

this.$$ = $({
verbose,
Expand All @@ -23,6 +24,14 @@ export class SshClient {
async ssh(command, options = {}) {
const $$ = options?.verbose ? $({ verbose: true }) : this.$$;

await $$`az ssh vm -n ${this.vmName} -g ${this.resourceGroupName} --yes -- -o StrictHostKeyChecking=accept-new ${command}`;
const target = `${this.username}@${this.ip}`;

await $$`ssh -i ${this.privateKeyPath} -o StrictHostKeyChecking=accept-new ${target} ${command}`;
}

async scp(source, destination) {
const target = `${this.username}@${this.ip}:${destination}`;
await this
.$$`scp -i ${this.privateKeyPath} -o StrictHostKeyChecking=accept-new ${source} ${target}`;
}
}
19 changes: 17 additions & 2 deletions packages/@n8n/benchmark/scripts/clients/terraformClient.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,24 @@ export class TerraformClient {
/**
* @typedef {Object} BenchmarkEnv
* @property {string} vmName
* @property {string} ip
* @property {string} sshUsername
* @property {string} sshPrivateKeyPath
*
* @returns {Promise<BenchmarkEnv>}
*/
async provisionEnvironment() {
console.log('Provisioning cloud environment...');

await this.$$`terraform init`;
await this.$$`terraform apply -input=false -auto-approve`;
// await this.$$`terraform apply -input=false -auto-approve`;

const privateKeyName = await this.extractPrivateKey();

return {
ip: await this.getTerraformOutput('ip'),
sshUsername: await this.getTerraformOutput('ssh_username'),
sshPrivateKeyPath: path.join(paths.infraCodeDir, privateKeyName),
vmName: await this.getTerraformOutput('vm_name'),
};
}
Expand All @@ -42,11 +50,18 @@ export class TerraformClient {

console.log('Destroying cloud environment...');

await this.$$`terraform destroy -input=false -auto-approve`;
// await this.$$`terraform destroy -input=false -auto-approve`;
}

async getTerraformOutput(key) {
const output = await this.$$`terraform output -raw ${key}`;
return output.stdout.trim();
}

async extractPrivateKey() {
await this.$$`terraform output -raw ssh_private_key > privatekey.pem`;
await this.$$`chmod 600 privatekey.pem`;

return 'privatekey.pem';
}
}
6 changes: 5 additions & 1 deletion packages/@n8n/benchmark/scripts/runForN8nSetup.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ async function main() {
const runDir = path.join(baseRunDir, n8nSetupToUse);
fs.emptyDirSync(runDir);

const currentUserId = process.getuid?.() ?? 0;
if (!process.getuid) {
console.error('Windows is not supported');
process.exit(1);
}

const currentUserId = process.getuid();
const dockerComposeClient = new DockerComposeClient({
$: $({
cwd: composeFilePath,
Expand Down
34 changes: 25 additions & 9 deletions packages/@n8n/benchmark/scripts/runInCloud.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@
* NOTE: Must be run in the root of the package.
*/
// @ts-check
import { sleep, which } from 'zx';
import { sleep, which, $, tmpdir } from 'zx';
import path from 'path';
import { SshClient } from './clients/sshClient.mjs';
import { TerraformClient } from './clients/terraformClient.mjs';

/**
* @typedef {Object} BenchmarkEnv
* @property {string} vmName
* @property {string} ip
* @property {string} sshUsername
* @property {string} sshPrivateKeyPath
*/

const RESOURCE_GROUP_NAME = 'n8n-benchmarking';

/**
* @typedef {Object} Config
* @property {boolean} isVerbose
Expand Down Expand Up @@ -63,14 +64,15 @@ async function runBenchmarksOnVm(config, benchmarkEnv) {
console.log(`Setting up the environment...`);

const sshClient = new SshClient({
vmName: benchmarkEnv.vmName,
resourceGroupName: RESOURCE_GROUP_NAME,
ip: benchmarkEnv.ip,
username: benchmarkEnv.sshUsername,
privateKeyPath: benchmarkEnv.sshPrivateKeyPath,
verbose: config.isVerbose,
});

await ensureVmIsReachable(sshClient);

const scriptsDir = await transferScriptsToVm(sshClient);
const scriptsDir = await transferScriptsToVm(sshClient, config);

// Bootstrap the environment with dependencies
console.log('Running bootstrap script...');
Expand Down Expand Up @@ -121,8 +123,22 @@ async function ensureVmIsReachable(sshClient) {
/**
* @returns Path where the scripts are located on the VM
*/
async function transferScriptsToVm(sshClient) {
await sshClient.ssh('rm -rf ~/n8n && git clone --depth=1 https://github.com/n8n-io/n8n.git');
async function transferScriptsToVm(sshClient, config) {
const cwd = process.cwd();
const scriptsDir = path.resolve(cwd, './scripts');
const tarFilename = 'scripts.tar.gz';
const scriptsTarPath = path.join(tmpdir('n8n-benchmark'), tarFilename);

const $$ = $({ verbose: config.isVerbose });

// Compress the scripts folder
await $$`tar -czf ${scriptsTarPath} ${scriptsDir} -C ${cwd} ./scripts`;

// Transfer the scripts to the VM
await sshClient.scp(scriptsTarPath, `~/${tarFilename}`);

// Extract the scripts on the VM
await sshClient.ssh(`tar -xzf ~/${tarFilename}`);

return '~/n8n/packages/@n8n/benchmark/scripts';
return '~/scripts';
}

0 comments on commit a03a797

Please sign in to comment.