Skip to content

Commit

Permalink
Add a hard timeout for all processes we spawn (#460)
Browse files Browse the repository at this point in the history
timeout kill all exec processes. default time is 10s
  • Loading branch information
juozasg authored Aug 9, 2023
1 parent 00f850b commit 2ca5aec
Show file tree
Hide file tree
Showing 22 changed files with 121 additions and 89 deletions.
13 changes: 8 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,15 @@
"default": false,
"description": "Enable WGE GitOpsTemplates feature"
},
"gitops.kubectlTimeout": {
"gitops.kubectlRequestTimeout": {
"type": "string",
"default": "10s",
"description": "kubectl --request-timeout"
},
"gitops.execTimeout": {
"type": "string",
"default": "0",
"description": "Seconds until SIGKILL for every shell exec (except `kubectl proxy`). Set to 0 for no timeout."
}
}
},
Expand Down Expand Up @@ -613,10 +618,10 @@
"eslint": "^8.11.0",
"glob": "^7.2.0",
"mocha": "^9.2.2",
"tough-cookie": ">=4.1.3",
"ts-loader": "^9.2.8",
"tsconfig-paths-webpack-plugin": "^4.0.1",
"typescript": "^4.5.5",
"tough-cookie": ">=4.1.3",
"vite": ">=2.9.16",
"webpack": "^5.70.0",
"webpack-cli": "^4.9.2",
Expand Down Expand Up @@ -644,9 +649,7 @@
"vscode-uri": "^3.0.7",
"word-wrap": ">=1.2.4"
},
"activationEvents": [
"onDebug"
],
"activationEvents": [],
"__metadata": {
"id": "61a914ed-c714-4c42-a201-6008038286a4",
"publisherDisplayName": "Weaveworks",
Expand Down
2 changes: 1 addition & 1 deletion src/cli/azure/azurePrereqs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { window } from 'vscode';

import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { ClusterProvider } from 'types/kubernetes/clusterProvider';
import { AzureClusterProvider } from './azureTools';

Expand Down
9 changes: 4 additions & 5 deletions src/cli/azure/azureTools.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import { Uri, env, window } from 'vscode';

import { fluxTools } from 'cli/flux/fluxTools';
import { ShellResult, shell, shellCodeError } from 'cli/shell/exec';
import { kubeConfig } from 'cli/kubernetes/kubernetesConfig';
import * as shell from 'cli/shell/exec';
import { ShellResult, shellCodeError } from 'cli/shell/exec';
import { refreshAllTreeViewsCommand } from 'commands/refreshTreeViews';
import { ClusterMetadata } from 'data/globalState';
import { globalState, telemetry } from 'extension';
import { failed } from 'types/errorable';
import { ClusterProvider } from 'types/kubernetes/clusterProvider';
import { TelemetryError } from 'types/telemetryEventNames';
import { getCurrentClusterInfo } from 'ui/treeviews/treeViews';
import { refreshAllTreeViewsCommand } from 'commands/refreshTreeViews';
import { parseJson } from 'utils/jsonUtils';
import { checkAzurePrerequisites } from './azurePrereqs';
import { getAzureMetadata } from './getAzureMetadata';
import { kubeConfig } from 'cli/kubernetes/kubernetesConfig';

export type AzureClusterProvider = ClusterProvider.AKS | ClusterProvider.AzureARC;

Expand Down
5 changes: 3 additions & 2 deletions src/cli/azure/getAzureMetadata.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import safesh from 'shell-escape-tag';
import { QuickPickItem, window } from 'vscode';

import { kubeConfig } from 'cli/kubernetes/kubernetesConfig';
import { invokeKubectlCommand } from 'cli/kubernetes/kubernetesToolsKubectl';
import { ShellResult, shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { ShellResult } from 'cli/shell/exec';
import { ClusterProvider } from 'types/kubernetes/clusterProvider';
import { ConfigMap } from 'types/kubernetes/kubernetesTypes';
import { parseJson } from 'utils/jsonUtils';
import { AzureClusterProvider, AzureConstants } from './azureTools';
import { kubeConfig } from 'cli/kubernetes/kubernetesConfig';

export interface AzureMetadata {
resourceGroup: string;
Expand Down
5 changes: 3 additions & 2 deletions src/cli/checkVersions.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { commands, Uri, window } from 'vscode';

import * as shell from 'cli/shell/exec';
import { enabledWGE, telemetry } from 'extension';
import { Errorable, failed } from 'types/errorable';
import { CommandId } from 'types/extensionIds';
import { TelemetryError } from 'types/telemetryEventNames';
import { parseJson } from 'utils/jsonUtils';
import { shell, shellCodeError } from './shell/exec';
import { clusterDataProvider } from 'ui/treeviews/treeViews';
import { parseJson } from 'utils/jsonUtils';
import { shellCodeError } from './shell/exec';

interface KubectlVersion {
major: string;
Expand Down
4 changes: 2 additions & 2 deletions src/cli/flux/fluxTools.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import safesh from 'shell-escape-tag';
import { window } from 'vscode';

import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { telemetry } from 'extension';
import { FluxSource, FluxTreeResources, FluxWorkload } from 'types/fluxCliTypes';
import { TelemetryError } from 'types/telemetryEventNames';
Expand Down Expand Up @@ -134,7 +134,7 @@ class FluxTools {

if (treeShellResult.code !== 0) {
telemetry.sendError(TelemetryError.FAILED_TO_RUN_FLUX_TREE);
window.showErrorMessage(`Failed to get resources created by the workload ${name}. ERROR: ${treeShellResult?.stderr}`);
window.showErrorMessage(`Failed to get resources created by the kustomization ${name}. ERROR: ${treeShellResult?.stderr}`);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cli/git/gitInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'path';
import { window } from 'vscode';

import { checkGitVersion } from 'cli/checkVersions';
import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { makeSSHUrlFromGitUrl } from 'commands/createSource';
import { GitRepository } from 'types/flux/gitRepository';
import { getGitRepositories } from 'cli/kubernetes/kubectlGet';
Expand Down
10 changes: 0 additions & 10 deletions src/cli/kubernetes/clusterProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,3 @@ async function isClusterAzureARC(context: string): Promise<ClusterProvider> {
return ClusterProvider.Generic;
}

/**
* Return true if gitops is enabled in the current cluster.
* Function checks if `flux-system` namespace contains flux controllers.
* @param contextName target cluster name
*/
export async function isGitOpsEnabled(contextName: string) {
const fluxControllers = await getFluxControllers(contextName);

return fluxControllers.length !== 0;
}
2 changes: 1 addition & 1 deletion src/cli/kubernetes/kubectlProxy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as k8s from '@kubernetes/client-node';
import { ChildProcess } from 'child_process';
import { kubeConfig } from 'cli/kubernetes/kubernetesConfig';
import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { createK8sClients, destroyK8sClients } from 'k8s/client';
import { createProxyConfig } from 'k8s/createKubeProxyConfig';

Expand Down
5 changes: 3 additions & 2 deletions src/cli/kubernetes/kubernetesConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,20 @@ export async function syncKubeConfig(forceReloadResourceKinds = false) {
const newKubeConfig = new k8s.KubeConfig();
newKubeConfig.loadFromString(configShellResult.stdout, {onInvalidEntry: ActionOnInvalid.FILTER});


if (kcTextChanged(kubeConfig, newKubeConfig)) {
const contextsListChanged = kcContextsListChanged(kubeConfig, newKubeConfig);
const contextChanged = kcCurrentContextChanged(kubeConfig, newKubeConfig);

// load the changed kubeconfig globally so that subsequent commands use the new config
// load the changed kubeconfig globally so that the following code use the new config
kubeConfig.loadFromString(configShellResult.stdout);

if(contextsListChanged) {
refreshClustersTreeView();
}

if(contextChanged || forceReloadResourceKinds) {
await loadAvailableResourceKinds();
loadAvailableResourceKinds();
}

if(contextChanged) {
Expand Down
1 change: 0 additions & 1 deletion src/cli/kubernetes/kubernetesConfigWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export async function initKubeConfigWatcher() {
});

restartFsWatcher();

}


Expand Down
10 changes: 7 additions & 3 deletions src/cli/kubernetes/kubernetesToolsKubectl.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { window, workspace } from 'vscode';
import * as kubernetes from 'vscode-kubernetes-tools-api';

import * as shell from 'cli/shell/exec';
import { output } from 'cli/shell/output';
import { telemetry } from 'extension';
import { TelemetryError } from 'types/telemetryEventNames';
Expand Down Expand Up @@ -45,8 +46,11 @@ export async function invokeKubectlCommand(command: string, printOutput = true):
}

let kubectlShellResult;
const commandWithArgs = `${command} --request-timeout ${getRequestTimeout()}`;
kubectlShellResult = await kubectl.invokeCommand(commandWithArgs);
const commandWithArgs = `kubectl ${command} --request-timeout ${getRequestTimeout()}`;
const t1 = Date.now();
kubectlShellResult = await shell.exec(commandWithArgs);
const t2 = Date.now();
console.log(`${command} ∆`, t2 - t1);


if(printOutput) {
Expand Down Expand Up @@ -75,6 +79,6 @@ export async function invokeKubectlCommand(command: string, printOutput = true):


function getRequestTimeout(): string {
return workspace.getConfiguration('gitops').get('kubectlTimeout') || '20s';
return workspace.getConfiguration('gitops').get('kubectlRequestTimeout') || '20s';
}

39 changes: 24 additions & 15 deletions src/cli/shell/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,22 @@ export type ShellHandler = (code: number, stdout: string, stderr: string)=> void
/**
* Return `true` when user has windows OS.
*/
function isWindows(): boolean {
export function isWindows(): boolean {
return (process.platform === WINDOWS) && !getUseWsl();
}

/**
* Return `true` when user has Unix OS.
*/
function isUnix(): boolean {
export function isUnix(): boolean {
return !isWindows();
}

/**
* Return user platform.
* For WSL - return Linux.
*/
function platform(): Platform {
export function platform(): Platform {
if (getUseWsl()) {
return Platform.Linux;
}
Expand Down Expand Up @@ -115,7 +115,9 @@ function execOpts({ cwd }: { cwd?: string; } = {}): shelljs.ExecOptions {
return opts;
}

async function exec(cmd: string, { cwd, callback }: { cwd?: string; callback?: ProcCallback;} = {}): Promise<ShellResult> {
export async function exec(
cmd: string,
{ cwd, callback }: { cwd?: string; callback?: ProcCallback; } = {}): Promise<ShellResult> {
try {
return await execCore(cmd, execOpts({ cwd }), callback);
} catch (e) {
Expand All @@ -133,7 +135,7 @@ async function exec(cmd: string, { cwd, callback }: { cwd?: string; callback?: P
* Execute command in cli and send the text to vscode output view.
* @param cmd CLI command string
*/
async function execWithOutput(
export async function execWithOutput(
cmd: string,
{
revealOutputView = true,
Expand Down Expand Up @@ -168,6 +170,7 @@ async function execWithOutput(
cwd: cwd,
env: execOpts().env,
});
setExecTimeoutKill(childProcess);

let stdout = '';
let stderr = '';
Expand Down Expand Up @@ -206,6 +209,7 @@ function execCore(cmd: string, opts: any, callback?: ProcCallback, stdin?: strin
cmd = `wsl ${cmd}`;
}
const proc = shelljs.exec(cmd, opts, (code, stdout, stderr) => resolve({code : code, stdout : stdout, stderr : stderr}));
setExecTimeoutKill(proc);
if (stdin) {
proc.stdin?.end(stdin);
}
Expand All @@ -215,7 +219,7 @@ function execCore(cmd: string, opts: any, callback?: ProcCallback, stdin?: strin
});
}

function execProc(cmd: string): ChildProcess {
export function execProc(cmd: string): ChildProcess {
const opts = execOpts();
if (getUseWsl()) {
cmd = `wsl ${cmd}`;
Expand All @@ -225,12 +229,17 @@ function execProc(cmd: string): ChildProcess {
return proc;
}

export const shell = {
isWindows : isWindows,
isUnix : isUnix,
platform : platform,
exec : exec,
execProc: execProc,
// execCore : execCore,
execWithOutput: execWithOutput,
};
function setExecTimeoutKill(proc: ChildProcess) {
const timeout = workspace.getConfiguration('gitops').get('execTimeout') as string;
const timeoutSeconds = parseInt(timeout);
if (isNaN(timeoutSeconds) || timeoutSeconds === 0) {
return;
}

setTimeout(() => {
if (proc.exitCode === null) {
proc.kill(9);
}
}, timeoutSeconds * 1000);
}

2 changes: 1 addition & 1 deletion src/commands/fluxCheck.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import safesh from 'shell-escape-tag';

import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { ClusterNode } from 'ui/treeviews/nodes/cluster/clusterNode';

/**
Expand Down
2 changes: 1 addition & 1 deletion src/commands/fluxCheckPrerequisites.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';

/**
* Runs `flux check --pre` command in the output view.
Expand Down
5 changes: 3 additions & 2 deletions src/commands/installFluxCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import path from 'path';
import request from 'request';
import { commands, window } from 'vscode';

import { Platform, shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { Platform } from 'cli/shell/exec';
import { output } from 'cli/shell/output';
import { runTerminalCommand } from 'cli/shell/terminal';
import { refreshAllTreeViewsCommand } from 'commands/refreshTreeViews';
import { GlobalStateKey } from 'data/globalState';
import { globalState } from 'extension';
import { Errorable, failed } from 'types/errorable';
import { refreshAllTreeViewsCommand } from 'commands/refreshTreeViews';
import { appendToPathEnvironmentVariableWindows, createDir, deleteFile, downloadFile, getAppdataPath, moveFile, readFile, unzipFile } from 'utils/fsUtils';

const fluxGitHubUserProject = 'fluxcd/flux2';
Expand Down
2 changes: 1 addition & 1 deletion src/commands/pullGitRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import safesh from 'shell-escape-tag';
import { commands, Uri, window } from 'vscode';

import { checkGitVersion } from 'cli/checkVersions';
import { shell } from 'cli/shell/exec';
import * as shell from 'cli/shell/exec';
import { telemetry } from 'extension';
import { TelemetryError } from 'types/telemetryEventNames';
import { GitRepositoryNode } from 'ui/treeviews/nodes/source/gitRepositoryNode';
Expand Down
Loading

0 comments on commit 2ca5aec

Please sign in to comment.