Skip to content

Commit e382660

Browse files
lstocchilstocchi
authored
fix: handle endless command execution (redhat-developer#730)
* refactor: clean check cluster status Signed-off-by: Luca Stocchi <[email protected]> * handle timed out error if cluster is not responding Signed-off-by: Luca Stocchi <[email protected]> * clean Signed-off-by: Luca Stocchi <[email protected]> * fix tests Signed-off-by: Luca Stocchi <[email protected]> * add timeout value as constants Signed-off-by: Luca Stocchi <[email protected]> Signed-off-by: Luca Stocchi <[email protected]>
1 parent a39c0ac commit e382660

22 files changed

+346
-189
lines changed

‎package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -677,12 +677,12 @@
677677
{
678678
"view": "tektonPipelineExplorerView",
679679
"contents": "Cluster is not accessible. Use install tool to Start Cluster or any other cluster which is active. \n[Kind](https://kubernetes.io/docs/tasks/tools/#kind)\n[Minikube](https://kubernetes.io/docs/tasks/tools/#minikube)\n[Red Hat OpenShift Local](https://developers.redhat.com/products/openshift-local/overview)",
680-
"when": "tekton.cluster"
680+
"when": "tekton.cluster.inactive"
681681
},
682682
{
683683
"view": "tektonPipelineExplorerView",
684-
"contents": "Tekton Pipeline Operator is not installed. Click the below button to see the documentation for getting started.\n[Tekton pipeline](https://tekton.dev/docs/getting-started/)\n[Tekton Trigger](https://tekton.dev/docs/triggers/install/)",
685-
"when": "tekton.pipeline"
684+
"contents": "Tekton Pipeline Operator is not installed. Click the below button to see the documentation for getting started.\n[Tekton pipeline](https://tekton.dev/docs/getting-started/)\n[Tekton Triggers](https://tekton.dev/docs/triggers/install/)",
685+
"when": "tekton.pipelines.inactive"
686686
}
687687
],
688688
"menus": {

‎src/check-cluster.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ export async function checkOpenShiftCluster(): Promise<clusterVersion> {
2525
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
2626
return JSON.parse(result?.stdout);
2727
}
28-
return null;
29-
} catch (err) {
30-
return null;
31-
}
28+
// eslint-disable-next-line no-empty
29+
} catch (err) {}
30+
return null
3231
}

‎src/cli-command.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,8 @@ export class Command {
232232
return newTknCommand('taskrun', 'logs', name, '-f');
233233
}
234234

235-
static checkTekton(): CliCommand {
236-
return newK8sCommand('auth', 'can-i', 'create', 'pipeline.tekton.dev', '&&', 'kubectl', 'get', 'pipeline.tekton.dev');
235+
static checkUserAuthentication(): CliCommand {
236+
return newK8sCommand('kubectl', 'get', 'pipeline.tekton.dev');
237237
}
238238

239239
static hubInstallTask(name: string, version: string): CliCommand {

‎src/cli.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import * as stream from 'stream';
1010
import * as JStream from 'jstream';
1111
import * as events from 'events';
1212
import { ToolsConfig } from './tools';
13+
import { DEFAULT_EXEC_TIMEOUT, ERR_CLUSTER_TIMED_OUT } from './constants';
1314

1415
export interface CliExitData {
1516
readonly error: string | Error;
@@ -110,7 +111,9 @@ export class CliImpl implements Cli {
110111
this.tknChannel.show();
111112
}
112113

113-
async execute(cmd: CliCommand, opts: SpawnOptions = {}): Promise<CliExitData> {
114+
async execute(cmd: CliCommand, opts: SpawnOptions = {
115+
timeout: DEFAULT_EXEC_TIMEOUT
116+
}): Promise<CliExitData> {
114117
return new Promise<CliExitData>((resolve) => {
115118
this.tknChannel.print(cliCommandToString(cmd));
116119
if (opts.windowsHide === undefined) {
@@ -121,7 +124,7 @@ export class CliImpl implements Cli {
121124
}
122125
const tkn = spawn(cmd.cliCommand, cmd.cliArguments, opts);
123126
let stdout = '';
124-
let error: string | Error;
127+
let error: string | Error = '';
125128
tkn.stdout.on('data', (data) => {
126129
stdout += data;
127130
});
@@ -136,6 +139,12 @@ export class CliImpl implements Cli {
136139
tkn.on('close', () => {
137140
resolve({ error, stdout });
138141
});
142+
if (opts.timeout > 0) {
143+
setTimeout(() => {
144+
error += ERR_CLUSTER_TIMED_OUT;
145+
tkn.kill();
146+
}, opts.timeout);
147+
}
139148
});
140149
}
141150

@@ -152,7 +161,7 @@ export class CliImpl implements Cli {
152161
}
153162

154163
executeWatchJSON(cmd: CliCommand, opts: SpawnOptions = {}): JSONWatchProcess {
155-
const toolLocation = ToolsConfig.getTknLocation(cmd.cliCommand);
164+
const toolLocation = ToolsConfig.getToolLocation(cmd.cliCommand);
156165
if (toolLocation) {
157166
// eslint-disable-next-line require-atomic-updates
158167
cmd.cliCommand = cmd.cliCommand.replace(cmd.cliCommand, `"${toolLocation}"`).replace(new RegExp(`&& ${cmd.cliCommand}`, 'g'), `&& "${toolLocation}"`);

‎src/constants.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/*-----------------------------------------------------------------------------------------------
2+
* Copyright (c) Red Hat, Inc. All rights reserved.
3+
* Licensed under the MIT License. See LICENSE file in the project root for license information.
4+
*-----------------------------------------------------------------------------------------------*/
5+
export const IS_CLUSTER_INACTIVE = 'tekton.cluster.inactive';
6+
export const IS_TEKTON_PIPELINES_INACTIVE = 'tekton.pipelines.inactive';
7+
8+
export const ERR_CLUSTER_TIMED_OUT = 'the cluster took too long to respond';
9+
10+
export const DEFAULT_EXEC_TIMEOUT = 10000;
11+
export const CHECK_USER_AUTHENTICATION_TIMEOUT = 5000;

‎src/debugger/debug-tree-view.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export async function watchTaskRunContainer(resourceName: string, resourceType:
5454
for (const containerData of taskRunData.status.steps) {
5555
const debugTaskName = taskRunData.metadata.name;
5656
const checkDebugStatus = await tkn.execute(Command.isContainerStoppedOnDebug(containerData.container, taskRunData.status.podName, taskRunData.metadata.namespace), process.cwd(), false);
57-
if (!debugSessions.get(debugTaskName)?.count && checkDebugStatus.stdout.trim() && checkDebugStatus.stdout.trim().length !== 0) {
57+
if (!debugSessions.get(debugTaskName)?.count && checkDebugStatus.stdout && checkDebugStatus.stdout.trim() && checkDebugStatus.stdout.trim().length !== 0) {
5858
tkn.executeInTerminal(Command.loginToContainer(containerData.container, taskRunData.status.podName, taskRunData.metadata.namespace), debugTaskName);
5959
debugSessions.set(debugTaskName, {
6060
count: true,

‎src/extension.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { TektonItem } from './tekton/tektonitem';
2222
import { showPipelinePreview, registerPipelinePreviewContext } from './pipeline/pipeline-preview';
2323
import { k8sCommands } from './kubernetes';
2424
import { initializeTknEditing } from './yaml-support/tkn-editing';
25-
import { ToolsConfig } from './tools';
25+
import { ToolConfigResult, ToolsConfig } from './tools';
2626
import { TKN_RESOURCE_SCHEME, TKN_RESOURCE_SCHEME_READONLY, tektonVfsProvider } from './util/tekton-vfs';
2727
import { updateTektonResource } from './tekton/deploy';
2828
import { deleteFromExplorer, deleteFromCustom } from './commands/delete';
@@ -48,6 +48,7 @@ import { openContainerInTerminal } from './debugger/show-in-terminal';
4848
import { debugName, debugSessions, pipelineTriggerStatus } from './util/map-object';
4949
import { deleteDebugger } from './debugger/delete-debugger';
5050
import { bundleWizard } from './tekton/bundle-pipeline-task';
51+
import { ERR_CLUSTER_TIMED_OUT } from './constants';
5152

5253
export let contextGlobalState: vscode.ExtensionContext;
5354
let k8sExplorer: k8s.ClusterExplorerV1 | undefined = undefined;
@@ -136,9 +137,9 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
136137
detectTknCli().then(() => {
137138
triggerDetection();
138139
});
139-
if (ToolsConfig.getTknLocation('kubectl')) {
140+
detectKubectlCli().then(() => {
140141
checkClusterStatus(true); // watch Tekton resources when all required dependency are installed
141-
}
142+
});
142143
getClusterVersions().then((version) => {
143144
const telemetryProps: TelemetryProperties = {
144145
identifier: 'cluster.version',
@@ -210,17 +211,36 @@ async function detectTknCli(): Promise<void> {
210211
setCommandContext(CommandContext.TknCli, false);
211212

212213
// start detecting 'tkn' on extension start
213-
const tknPath = await ToolsConfig.detectOrDownload('tkn');
214-
215-
if (tknPath) {
214+
const tknTool = await detectCli('tkn');
215+
if (tknTool) {
216216
setCommandContext(CommandContext.TknCli, true);
217-
sendVersionToTelemetry('tkn.version', tknPath);
218-
}
219-
if (ToolsConfig.getTknLocation('kubectl')) {
220-
sendVersionToTelemetry('kubectl.version', `${ToolsConfig.getTknLocation('kubectl')} -o json`);
217+
sendVersionToTelemetry('tkn.version', tknTool.location);
218+
}
219+
}
220+
221+
async function detectKubectlCli(): Promise<void> {
222+
const tool = await detectCli('kubectl');
223+
if (tool) {
224+
sendVersionToTelemetry('kubectl.version', `${ToolsConfig.getToolLocation('kubectl')} -o json`);
221225
}
222226
}
223227

228+
async function detectCli(cliName: string): Promise<ToolConfigResult> {
229+
const tool = await ToolsConfig.detectOrDownload(cliName);
230+
if (!tool) {
231+
await vscode.window.showInformationMessage(
232+
`Cannot find ${cliName} cli v${ToolsConfig.tool[cliName].version}. Please download it and try again`);
233+
return undefined;
234+
}
235+
236+
if (tool && tool.error === ERR_CLUSTER_TIMED_OUT) {
237+
await vscode.window.showWarningMessage(
238+
'The cluster took too long to respond. Please make sure the cluster is running and you can connect to it.');
239+
return undefined;
240+
}
241+
return tool;
242+
}
243+
224244
async function sendVersionToTelemetry(commandId: string, cmd: string): Promise<void> {
225245
const telemetryProps: TelemetryProperties = {
226246
identifier: commandId,
@@ -241,7 +261,7 @@ async function sendVersionToTelemetry(commandId: string, cmd: string): Promise<v
241261
} else {
242262
telemetryProps[key] = `v${value['major']}.${value['minor']}`;
243263
}
244-
}
264+
}
245265
sendTknAndKubectlVersionToTelemetry(commandId, telemetryProps);
246266
}
247267
}
@@ -296,7 +316,7 @@ function expandMoreItem(context: number, parent: TektonNode, treeViewId: string)
296316
}
297317

298318
function refreshTreeView(): void {
299-
if (ToolsConfig.getTknLocation('kubectl')) {
319+
if (ToolsConfig.getToolLocation('kubectl')) {
300320
checkClusterStatus(true);
301321
}
302322
pipelineExplorer.refresh();

‎src/tekton/bundle-pipeline-task.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import * as yaml from 'js-yaml';
1616
import { Platform } from '../util/platform';
1717
import { getStderrString } from '../util/stderrstring';
1818
import { clusterPipelineStatus } from '../util/map-object';
19+
import { IS_CLUSTER_INACTIVE, IS_TEKTON_PIPELINES_INACTIVE } from '../constants';
1920

2021
interface BundleType {
2122
imageDetail: string;
@@ -25,11 +26,11 @@ interface BundleType {
2526
}
2627

2728
export async function bundleWizard(): Promise<void> {
28-
if (clusterPipelineStatus.get('tekton.cluster')) {
29+
if (clusterPipelineStatus.get(IS_CLUSTER_INACTIVE)) {
2930
window.showWarningMessage('Please check that your cluster is working fine and try again.');
3031
return;
3132
}
32-
if (clusterPipelineStatus.get('tekton.pipeline')) {
33+
if (clusterPipelineStatus.get(IS_TEKTON_PIPELINES_INACTIVE)) {
3334
window.showWarningMessage('Please install the Pipelines Operator.');
3435
return;
3536
}

‎src/tekton/diagnostic.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ export async function showDiagnosticData(diagnostic: TektonNode, commandId?: str
2222
window.showInformationMessage(`No data available for ${diagnostic.getName()} to Diagnostic Data.`);
2323
return;
2424
}
25+
if (!result || result.error) {
26+
return;
27+
}
2528
const data = JSON.parse(result.stdout);
2629
if (diagnostic.contextValue === ContextType.PIPELINERUN || diagnostic.contextValue === ContextType.PIPELINERUNCHILDNODE) {
2730
const taskRun = [];

‎src/tkn.ts

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import { getPipelineList } from './util/list-tekton-resource';
2323
import { telemetryLog, telemetryLogError } from './telemetry';
2424
import { checkClusterStatus } from './util/check-cluster-status';
2525
import { treeRefresh } from './util/watchResources';
26+
import { SpawnOptions } from 'child_process';
27+
import { ERR_CLUSTER_TIMED_OUT } from './constants';
2628

2729

2830
const tektonResourceCount = {};
@@ -72,6 +74,7 @@ export interface Tkn {
7274
getRawTasks(): Promise<TknTask[]>;
7375
getClusterTasks(clustertask?: TektonNode): Promise<TektonNode[]>;
7476
getRawClusterTasks(): Promise<TknTask[]>;
77+
executeWithOptions(command: CliCommand, opts?: SpawnOptions, fail?: boolean): Promise<CliExitData>;
7578
execute(command: CliCommand, cwd?: string, fail?: boolean): Promise<CliExitData>;
7679
// eslint-disable-next-line @typescript-eslint/ban-types
7780
executeWatch(command: CliCommand, opts?: {}): WatchProcess;
@@ -408,7 +411,7 @@ export class TknImpl implements Tkn {
408411
private async _getTriggerResource(trigerResource: TektonNode, command: CliCommand, triggerContextType: ContextType, triggerType: string): Promise<TektonNode[]> {
409412
let data: TknPipelineResource[] = [];
410413
const result = await this.execute(command, process.cwd(), false);
411-
const triggerCheck = RegExp('undefinederror: the server doesn\'t have a resource type');
414+
const triggerCheck = RegExp('error: the server doesn\'t have a resource type');
412415
if (triggerCheck.test(getStderrString(result.error))) {
413416
telemetryLogError(`tekton.list.${triggerType}`, result.error);
414417
return;
@@ -465,7 +468,7 @@ export class TknImpl implements Tkn {
465468

466469
async getRawTasks(): Promise<TknTask[]> {
467470
let data: TknTask[] = [];
468-
if (!ToolsConfig.getTknLocation('kubectl')) return null;
471+
if (!ToolsConfig.getToolLocation('kubectl')) return null;
469472
const result = await this.execute(Command.listTasks());
470473
if (result.error) {
471474
console.error(result + 'Std.err when processing tasks');
@@ -525,7 +528,7 @@ export class TknImpl implements Tkn {
525528

526529
async getRawClusterTasks(): Promise<TknTask[]> {
527530
let data: TknTask[] = [];
528-
if (!ToolsConfig.getTknLocation('kubectl')) return null;
531+
if (!ToolsConfig.getToolLocation('kubectl')) return null;
529532
const result = await this.execute(Command.listClusterTasks());
530533
if (result.error) {
531534
console.log(result + 'Std.err when processing tasks');
@@ -561,10 +564,11 @@ export class TknImpl implements Tkn {
561564
}
562565

563566
async executeInTerminal(command: CliCommand, resourceName?: string, cwd: string = process.cwd(), name = 'Tekton'): Promise<void> {
564-
let toolLocation = await ToolsConfig.detectOrDownload(command.cliCommand);
565-
if (toolLocation) {
566-
toolLocation = path.dirname(toolLocation);
567+
const toolConfig = await ToolsConfig.detectOrDownload(command.cliCommand);
568+
if (!toolConfig || toolConfig.error == ERR_CLUSTER_TIMED_OUT) {
569+
return;
567570
}
571+
const toolLocation = path.dirname(toolConfig.location);
568572
let terminal: Terminal;
569573
if (resourceName) {
570574
terminal = WindowUtil.createTerminal(`${name}:${resourceName}`, cwd, toolLocation);
@@ -575,28 +579,46 @@ export class TknImpl implements Tkn {
575579
terminal.show();
576580
}
577581

582+
async executeWithOptions(command: CliCommand, opts?: SpawnOptions, fail?: boolean): Promise<CliExitData> {
583+
const cleanedCommand = await this.createCliCommand(command);
584+
if (typeof cleanedCommand === 'string') {
585+
return {
586+
error: cleanedCommand,
587+
stdout: undefined
588+
};
589+
}
590+
591+
return cli.execute(command, opts ? opts : {})
592+
.then(async (result) => result.error && fail ? Promise.reject(result.error) : result)
593+
.catch((err) => fail ? Promise.reject(err) : Promise.resolve({ error: null, stdout: '', stderr: '' }));
594+
}
595+
578596
async execute(command: CliCommand, cwd?: string, fail = true): Promise<CliExitData> {
597+
return this.executeWithOptions(command, cwd ? { cwd } : {}, fail);
598+
}
599+
600+
async createCliCommand(command: CliCommand): Promise<CliCommand | string> {
579601
if (command.cliCommand.indexOf('tkn') >= 0) {
580-
const toolLocation = ToolsConfig.getTknLocation('tkn');
602+
const toolLocation = ToolsConfig.getToolLocation('tkn');
581603
if (toolLocation) {
582-
// eslint-disable-next-line require-atomic-updates
583604
command.cliCommand = command.cliCommand.replace('tkn', `"${toolLocation}"`).replace(new RegExp('&& tkn', 'g'), `&& "${toolLocation}"`);
584605
}
585606
} else {
586-
const toolLocation = await ToolsConfig.detectOrDownload(command.cliCommand);
587-
if (toolLocation) {
588-
// eslint-disable-next-line require-atomic-updates
589-
command.cliCommand = command.cliCommand.replace(command.cliCommand, `"${toolLocation}"`).replace(new RegExp(`&& ${command.cliCommand}`, 'g'), `&& "${toolLocation}"`);
607+
const toolConfig = await ToolsConfig.detectOrDownload(command.cliCommand);
608+
if (toolConfig) {
609+
if (toolConfig.error === ERR_CLUSTER_TIMED_OUT) {
610+
return toolConfig.error;
611+
}
612+
if (toolConfig.location) {
613+
command.cliCommand = command.cliCommand.replace(command.cliCommand, `"${toolConfig.location}"`).replace(new RegExp(`&& ${command.cliCommand}`, 'g'), `&& "${toolConfig.location}"`);
614+
}
590615
}
591616
}
592-
593-
return cli.execute(command, cwd ? { cwd } : {})
594-
.then(async (result) => result.error && fail ? Promise.reject(result.error) : result)
595-
.catch((err) => fail ? Promise.reject(err) : Promise.resolve({ error: null, stdout: '', stderr: '' }));
617+
return command;
596618
}
597619

598620
executeWatch(command: CliCommand, cwd?: string): WatchProcess {
599-
const toolLocation = ToolsConfig.getTknLocation(command.cliCommand);
621+
const toolLocation = ToolsConfig.getToolLocation(command.cliCommand);
600622
if (toolLocation) {
601623
// eslint-disable-next-line require-atomic-updates
602624
command.cliCommand = command.cliCommand.replace(command.cliCommand, `"${toolLocation}"`).replace(new RegExp(`&& ${command.cliCommand}`, 'g'), `&& "${toolLocation}"`);

0 commit comments

Comments
 (0)