From 738ca479e49313e4c1b09789a3b37a952ca867d7 Mon Sep 17 00:00:00 2001 From: Arvind Ranasaria Date: Fri, 8 Nov 2019 09:11:21 -0800 Subject: [PATCH] fixes for #8165, #8167, & #8260 (#8250) * saving untested work * fixes for #8165 and #8167 * minor fixes * fix for #8260 * minor quoting fixes * fix for #8264 * minor fixes * minor fixes. * move tools constants to their own files * remove execution cell results from notebooks. * remove extraneous changes * move ensuring of StoragePath to platformservice * remove fix for #8264 pending pm input --- .../notebooks/bdc/2019/deploy-bdc-aks.ipynb | 4 ++-- .../bdc/2019/deploy-bdc-existing-aks.ipynb | 4 ++-- .../2019/deploy-bdc-existing-kubeadm.ipynb | 2 +- extensions/resource-deployment/src/main.ts | 3 ++- .../src/services/platformService.ts | 7 +++++++ .../src/services/tools/azCliTool.ts | 5 +++-- .../src/services/tools/azdataTool.ts | 3 ++- .../src/services/tools/kubeCtlTool.ts | 5 +++-- .../src/services/tools/toolBase.ts | 21 +++++++------------ .../deployClusterWizardModel.ts | 14 ++++++++----- .../resource-deployment/src/ui/model.ts | 3 ++- extensions/resource-deployment/src/utils.ts | 10 +++++++-- 12 files changed, 49 insertions(+), 32 deletions(-) diff --git a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-aks.ipynb b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-aks.ipynb index 9c52dff8df..c93c2c333e 100644 --- a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-aks.ipynb +++ b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-aks.ipynb @@ -331,7 +331,7 @@ "os.environ[\"AZDATA_USERNAME\"] = mssql_username\n", "os.environ[\"AZDATA_PASSWORD\"] = mssql_password\n", "if os.name == 'nt':\n", - " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\tkubectl get pods -n {mssql_cluster_name} ')\n", + " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\t{os.environ[\"AZDATA_NB_VAR_KUBECTL\"]} get pods -n {mssql_cluster_name} ')\n", "run_command(f'azdata bdc create -c {mssql_target_profile}')" ], "metadata": { @@ -426,4 +426,4 @@ "execution_count": 14 } ] -} \ No newline at end of file +} diff --git a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-aks.ipynb b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-aks.ipynb index b315356551..3e524261fe 100644 --- a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-aks.ipynb +++ b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-aks.ipynb @@ -230,7 +230,7 @@ "os.environ[\"AZDATA_USERNAME\"] = mssql_username\n", "os.environ[\"AZDATA_PASSWORD\"] = mssql_password\n", "if os.name == 'nt':\n", - " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\tkubectl get pods -n {mssql_cluster_name} ')\n", + " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\t{os.environ[\"AZDATA_NB_VAR_KUBECTL\"]} get pods -n {mssql_cluster_name} ')\n", "run_command(f'azdata bdc create -c {mssql_target_profile}')" ], "metadata": { @@ -325,4 +325,4 @@ "execution_count": 10 } ] -} \ No newline at end of file +} diff --git a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-kubeadm.ipynb b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-kubeadm.ipynb index 9c2270ba7b..6bb47a6e65 100644 --- a/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-kubeadm.ipynb +++ b/extensions/resource-deployment/notebooks/bdc/2019/deploy-bdc-existing-kubeadm.ipynb @@ -236,7 +236,7 @@ " os.environ[\"DOMAIN_SERVICE_ACCOUNT_USERNAME\"] = mssql_domain_service_account_username\n", " os.environ[\"DOMAIN_SERVICE_ACCOUNT_PASSWORD\"] = mssql_domain_service_account_password\n", "if os.name == 'nt':\n", - " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\tkubectl get pods -n {mssql_cluster_name} ')\n", + " print(f'If you don\\'t see output produced by azdata, you can run the following command in a terminal window to check the deployment status:\\n\\t{os.environ[\"AZDATA_NB_VAR_KUBECTL\"]} get pods -n {mssql_cluster_name} ')\n", "run_command(f'azdata bdc create -c {mssql_target_profile}')" ], "metadata": { diff --git a/extensions/resource-deployment/src/main.ts b/extensions/resource-deployment/src/main.ts index c6bf8704cf..b1ed3fdafc 100644 --- a/extensions/resource-deployment/src/main.ts +++ b/extensions/resource-deployment/src/main.ts @@ -15,8 +15,9 @@ import { ResourceTypePickerDialog } from './ui/resourceTypePickerDialog'; const localize = nls.loadMessageBundle(); -export function activate(context: vscode.ExtensionContext) { +export async function activate(context: vscode.ExtensionContext): Promise { const platformService = new PlatformService(context.globalStoragePath); + await platformService.initialize(); const toolsService = new ToolsService(platformService); const notebookService = new NotebookService(platformService, context.extensionPath); const resourceTypeService = new ResourceTypeService(platformService, toolsService, notebookService); diff --git a/extensions/resource-deployment/src/services/platformService.ts b/extensions/resource-deployment/src/services/platformService.ts index 0180265096..85886b6979 100644 --- a/extensions/resource-deployment/src/services/platformService.ts +++ b/extensions/resource-deployment/src/services/platformService.ts @@ -21,6 +21,7 @@ export interface IPlatformService { osType(): OsType; platform(): string; storagePath(): string; + initialize(): Promise; copyFile(source: string, target: string): Promise; fileExists(file: string): Promise; openFile(filePath: string): void; @@ -56,8 +57,14 @@ export interface CommandOptions { export class PlatformService implements IPlatformService { private _outputChannel: vscode.OutputChannel = vscode.window.createOutputChannel(extensionOutputChannel); + private _storagePathEnsurer: Promise; constructor(private _storagePath: string = '') { + this._storagePathEnsurer = this.ensureDirectoryExists(_storagePath); + } + + async initialize(): Promise { + await this._storagePathEnsurer; } storagePath(): string { diff --git a/extensions/resource-deployment/src/services/tools/azCliTool.ts b/extensions/resource-deployment/src/services/tools/azCliTool.ts index a4b3125fb3..70284f260e 100644 --- a/extensions/resource-deployment/src/services/tools/azCliTool.ts +++ b/extensions/resource-deployment/src/services/tools/azCliTool.ts @@ -7,11 +7,12 @@ import { SemVer } from 'semver'; import * as nls from 'vscode-nls'; import { Command, OsType, ToolType } from '../../interfaces'; import { IPlatformService } from '../platformService'; -import { ToolBase, dependencyType } from './toolBase'; +import { dependencyType, ToolBase } from './toolBase'; const localize = nls.loadMessageBundle(); const defaultInstallationRoot = '~/.local/bin'; const win32InstallationRoot = `${process.env['ProgramFiles(x86)']}\\Microsoft SDKs\\Azure\\CLI2\\wbin`; +export const AzCliToolName = 'azure-cli'; export class AzCliTool extends ToolBase { constructor(platformService: IPlatformService) { @@ -19,7 +20,7 @@ export class AzCliTool extends ToolBase { } get name(): string { - return 'azure-cli'; + return AzCliToolName; } get description(): string { diff --git a/extensions/resource-deployment/src/services/tools/azdataTool.ts b/extensions/resource-deployment/src/services/tools/azdataTool.ts index 6f3cd1b211..9769b10b5a 100644 --- a/extensions/resource-deployment/src/services/tools/azdataTool.ts +++ b/extensions/resource-deployment/src/services/tools/azdataTool.ts @@ -13,6 +13,7 @@ import { IPlatformService } from '../platformService'; import { dependencyType, ToolBase } from './toolBase'; const localize = nls.loadMessageBundle(); +export const AzdataToolName = 'azdata'; export class AzdataTool extends ToolBase { constructor(platformService: IPlatformService) { @@ -20,7 +21,7 @@ export class AzdataTool extends ToolBase { } get name(): string { - return 'azdata'; + return AzdataToolName; } get description(): string { diff --git a/extensions/resource-deployment/src/services/tools/kubeCtlTool.ts b/extensions/resource-deployment/src/services/tools/kubeCtlTool.ts index caa02adabc..f3281bf9aa 100644 --- a/extensions/resource-deployment/src/services/tools/kubeCtlTool.ts +++ b/extensions/resource-deployment/src/services/tools/kubeCtlTool.ts @@ -7,11 +7,12 @@ import { Command, ToolType, OsType } from '../../interfaces'; import * as nls from 'vscode-nls'; import { SemVer } from 'semver'; import { IPlatformService } from '../platformService'; -import { ToolBase, dependencyType } from './toolBase'; +import { dependencyType, ToolBase } from './toolBase'; const localize = nls.loadMessageBundle(); const defaultInstallationRoot = '/usr/local/bin'; +export const KubeCtlToolName = 'kubectl'; export class KubeCtlTool extends ToolBase { constructor(platformService: IPlatformService) { @@ -19,7 +20,7 @@ export class KubeCtlTool extends ToolBase { } get name(): string { - return 'kubectl'; + return KubeCtlToolName; } get description(): string { diff --git a/extensions/resource-deployment/src/services/tools/toolBase.ts b/extensions/resource-deployment/src/services/tools/toolBase.ts index 6aef929e1c..08fc68141c 100644 --- a/extensions/resource-deployment/src/services/tools/toolBase.ts +++ b/extensions/resource-deployment/src/services/tools/toolBase.ts @@ -3,7 +3,7 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import { EOL } from 'os'; -import { delimiter } from 'path'; +import * as path from 'path'; import { SemVer, compare } from 'semver'; import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; @@ -32,7 +32,7 @@ export const enum dependencyType { Curl = 'Curl' } -const pythonAndPip3Localized = localize('deploymentDialog.ToolInformationalMessage.PythonAndPip3', "• azdata installation needs python3 and pip3 to be pre-installed before necessary tools can be deployed"); +const pythonAndPip3Localized = localize('deploymentDialog.ToolInformationalMessage.PythonAndPip3', "• azdata installation needs pip3 and python3 version 3.6 to be pre-installed before necessary tools can be deployed"); const brewLocalized = localize('deploymentDialog.ToolInformationalMessage.Brew', "• brew is needed for deployment of the tools and needs to be pre-installed before necessary tools can be deployed"); const curlLocalized = localize('deploymentDialog.ToolInformationalMessage.Curl', "• curl is needed for installation and needs to be pre-installed before necessary tools can be deployed"); @@ -122,12 +122,7 @@ export abstract class ToolBase implements ITool { } public get storagePath(): string { - const storagePath = this._platformService.storagePath(); - if (!this._storagePathEnsured) { - this._platformService.ensureDirectoryExists(storagePath); - this._storagePathEnsured = true; - } - return storagePath; + return this._platformService.storagePath(); } public get osType(): OsType { @@ -155,6 +150,7 @@ export abstract class ToolBase implements ITool { public get installationPath(): string { return this._installationPath; } + protected get installationCommands(): Command[] | undefined { return this.allInstallationCommands.get(this.osType); } @@ -232,9 +228,9 @@ export abstract class ToolBase implements ITool { this.logToOutputChannel(localize('toolBase.addInstallationSearchPathsToSystemPath.SearchPaths', "Search Paths for tool '{0}': {1}", this.displayName, JSON.stringify(searchPaths, undefined, '\t'))); //this.displayName is localized and searchPaths are OS filesystem paths. searchPaths.forEach(searchPath => { if (process.env.PATH) { - if (!`${delimiter}${process.env.PATH}${delimiter}`.includes(`${delimiter}${searchPath}${delimiter}`)) { - process.env.PATH += `${delimiter}${searchPath}`; - console.log(`Appending to Path -> '${delimiter}${searchPath}'`); + if (!`${path.delimiter}${process.env.PATH}${path.delimiter}`.includes(`${path.delimiter}${searchPath}${path.delimiter}`)) { + process.env.PATH += `${path.delimiter}${searchPath}`; + console.log(`Appending to Path -> '${path.delimiter}${searchPath}'`); } } else { process.env.PATH = searchPath; @@ -299,7 +295,7 @@ export abstract class ToolBase implements ITool { if (!commandOutput) { throw new Error(`Install location of tool:'${this.displayName}' could not be discovered`); } else { - this._installationPath = commandOutput.split(EOL)[0]; + this._installationPath = path.resolve(commandOutput.split(EOL)[0]); } } @@ -307,7 +303,6 @@ export abstract class ToolBase implements ITool { return this._version ? compare(this._version, new SemVer(version)) >= 0 : false; } - private _storagePathEnsured: boolean = false; private _status: ToolStatus = ToolStatus.NotInstalled; private _osType: OsType; private _version?: SemVer; diff --git a/extensions/resource-deployment/src/ui/deployClusterWizard/deployClusterWizardModel.ts b/extensions/resource-deployment/src/ui/deployClusterWizard/deployClusterWizardModel.ts index 9bfbad422c..c7d0a1a644 100644 --- a/extensions/resource-deployment/src/ui/deployClusterWizard/deployClusterWizardModel.ts +++ b/extensions/resource-deployment/src/ui/deployClusterWizard/deployClusterWizardModel.ts @@ -2,13 +2,15 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ - -import { Model } from '../model'; -import * as VariableNames from './constants'; -import { BigDataClusterDeploymentProfile, DataResource, SqlServerMasterResource, HdfsResource } from '../../services/bigDataClusterDeploymentProfile'; -import { BdcDeploymentType } from '../../interfaces'; import { EOL } from 'os'; import { delimiter } from 'path'; +import { BdcDeploymentType } from '../../interfaces'; +import { BigDataClusterDeploymentProfile, DataResource, HdfsResource, SqlServerMasterResource } from '../../services/bigDataClusterDeploymentProfile'; +import { KubeCtlToolName } from '../../services/tools/kubeCtlTool'; +import { getRuntimeBinaryPathEnvironmentVariableName } from '../../utils'; +import { Model } from '../model'; +import * as VariableNames from './constants'; + export class DeployClusterWizardModel extends Model { constructor(public deploymentTarget: BdcDeploymentType) { @@ -163,6 +165,8 @@ export class DeployClusterWizardModel extends Model { statements.push(`os.environ["DOCKER_USERNAME"] = '${this.getStringValue(VariableNames.DockerUsername_VariableName)}'`); statements.push(`os.environ["DOCKER_PASSWORD"] = os.environ["${VariableNames.DockerPassword_VariableName}"]`); } + const kubeCtlEnvVarName: string = getRuntimeBinaryPathEnvironmentVariableName(KubeCtlToolName); + statements.push(`os.environ["${kubeCtlEnvVarName}"] = "${this.escapeForNotebookCodeCell(process.env[kubeCtlEnvVarName]!)}"`); statements.push(`os.environ["PATH"] = os.environ["PATH"] + "${delimiter}" + "${this.escapeForNotebookCodeCell(process.env[VariableNames.ToolsInstallPath]!)}"`); statements.push(`print('Variables have been set successfully.')`); return statements.map(line => line + EOL); diff --git a/extensions/resource-deployment/src/ui/model.ts b/extensions/resource-deployment/src/ui/model.ts index 8c61b6adf3..0cf853fd38 100644 --- a/extensions/resource-deployment/src/ui/model.ts +++ b/extensions/resource-deployment/src/ui/model.ts @@ -2,6 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { NoteBookEnvironmentVariablePrefix } from '../interfaces'; export class Model { private propValueObject: { [s: string]: string | undefined } = {}; @@ -32,7 +33,7 @@ export class Model { } public setEnvironmentVariables(): void { - Object.keys(this.propValueObject).filter(propertyName => propertyName.startsWith('AZDATA_NB_VAR_')).forEach(propertyName => { + Object.keys(this.propValueObject).filter(propertyName => propertyName.startsWith(NoteBookEnvironmentVariablePrefix)).forEach(propertyName => { const value = this.getStringValue(propertyName); if (value !== undefined && value !== '') { process.env[propertyName] = value; diff --git a/extensions/resource-deployment/src/utils.ts b/extensions/resource-deployment/src/utils.ts index 9a407a5f53..7f78ce660a 100644 --- a/extensions/resource-deployment/src/utils.ts +++ b/extensions/resource-deployment/src/utils.ts @@ -16,16 +16,22 @@ export function getDateTimeString(): string { return new Date().toISOString().slice(0, 19).replace(/[^0-9]/g, ''); // Take the date time information and only leaving the numbers } + +export function getRuntimeBinaryPathEnvironmentVariableName(toolName: string): string { + return `${NoteBookEnvironmentVariablePrefix}${toolName.toUpperCase().replace(/ |-/g, '_')}`; +} + export function setEnvironmentVariablesForInstallPaths(tools: ITool[]): void { // Use Set class to make sure the collection only contains unique values. let installationPaths: Set = new Set(); tools.forEach(t => { if (t.installationPath) { + // construct an env variable name with NoteBookEnvironmentVariablePrefix prefix // and tool.name as suffix, making sure of using all uppercase characters and only _ as separator - const envVarName: string = `${NoteBookEnvironmentVariablePrefix}${t.name.toUpperCase().replace(/ |-/g, '_')}`; + const envVarName = getRuntimeBinaryPathEnvironmentVariableName(t.name); process.env[envVarName] = t.installationPath; - installationPaths.add(path.resolve(path.dirname(t.installationPath))); + installationPaths.add(path.dirname(t.installationPath)); console.log(`setting env var:'${envVarName}' to: '${t.installationPath}'`); } });