Canye/fix refresh (#19424)

* Bump version of arc and azcli to 1.3.0

* Fixed refresh to do dc config show and endpoint list first, then check the connectionmode

* Await calls to refresh indirect and direct

* Made sql mi-arc delete also handle direct mode. Made customlocation for sql mi create work again, regex problem.

Co-authored-by: Candice Ye <canye@microsoft.com>

extensions/arc/src/localizedConstants.ts

@@ -359,3 +359,4 @@ export function upgradingMiaa(param: any): string { return localize('arc.upgradi
 export const currentVersion = localize('arc.currentVersion', "Current version");
 export const showMiaaError = localize('arc.showMiaaError', "Error showing details of SQL managed instance.");
 export const miaaVersionError = localize('arc.miaaVersionError', "Error getting SQL managed instance version number.");
+export const errorGettingConnectionMode = localize('arc.errorGettingConnectionMode', "Error getting data controller connection mode.");

extensions/arc/src/models/controllerModel.ts

@@ -64,20 +64,20 @@ export class ControllerModel {
 		if (node) {
 			this.treeDataProvider.refreshNode(node);
 		} else {
-			await this.refresh(false, this.info.resourceGroup, this.info.namespace);
+			await this.refresh(false, this.info.namespace);
 		}
 	}
 
-	public async refresh(showErrors: boolean = true, resourceGroup: string, namespace: string): Promise<void> {
+	public async refresh(showErrors: boolean = true, namespace: string): Promise<void> {
-		if (this.info.connectionMode === ConnectionMode.direct) {
+		await this.refreshController(showErrors, namespace);
-			this.refreshDirectMode(showErrors, resourceGroup, namespace);
+		if (this._controllerConfig?.spec.settings.azure.connectionMode === ConnectionMode.direct) {
+			await this.refreshDirectMode(this._controllerConfig?.spec.settings.azure.resourceGroup, namespace);
 		} else {
-			this.refreshIndirectMode(showErrors, namespace);
+			await this.refreshIndirectMode(namespace);
 		}
 	}
 
-	public async refreshDirectMode(showErrors: boolean = true, resourceGroup: string, namespace: string): Promise<void> {
+	public async refreshController(showErrors: boolean = true, namespace: string): Promise<void> {
-		const newRegistrations: Registration[] = [];
 		await Promise.all([
 			this._azApi.az.arcdata.dc.config.show(namespace, this.azAdditionalEnvVars).then(result => {
 				this._controllerConfig = result.stdout;
@@ -106,8 +106,13 @@ export class ControllerModel {
 				}
 				this._onEndpointsUpdated.fire(this._endpoints);
 				throw err;
-			}),
+			})
-			Promise.all([
+		]);
+	}
+
+	public async refreshDirectMode(resourceGroup: string, namespace: string): Promise<void> {
+		const newRegistrations: Registration[] = [];
+		await Promise.all([
 			this._azApi.az.postgres.arcserver.list(namespace, this.azAdditionalEnvVars).then(result => {
 				newRegistrations.push(...result.stdout.map(r => {
 					return {
@@ -131,42 +136,12 @@ export class ControllerModel {
 			this._registrations = newRegistrations;
 			this.registrationsLastUpdated = new Date();
 			this._onRegistrationsUpdated.fire(this._registrations);
-			})
+		});
-		]);
 	}
 
-	public async refreshIndirectMode(showErrors: boolean = true, namespace: string): Promise<void> {
+	public async refreshIndirectMode(namespace: string): Promise<void> {
 		const newRegistrations: Registration[] = [];
 		await Promise.all([
-			this._azApi.az.arcdata.dc.config.show(namespace, this.azAdditionalEnvVars).then(result => {
-				this._controllerConfig = result.stdout;
-				this.configLastUpdated = new Date();
-				this._onConfigUpdated.fire(this._controllerConfig);
-			}).catch(err => {
-				// If an error occurs show a message so the user knows something failed but still
-				// fire the event so callers hooking into this can handle the error (e.g. so dashboards don't show the
-				// loading icon forever)
-				if (showErrors) {
-					vscode.window.showErrorMessage(loc.fetchConfigFailed(this.info.name, err));
-				}
-				this._onConfigUpdated.fire(this._controllerConfig);
-				throw err;
-			}),
-			this._azApi.az.arcdata.dc.endpoint.list(namespace, this.azAdditionalEnvVars).then(result => {
-				this._endpoints = result.stdout;
-				this.endpointsLastUpdated = new Date();
-				this._onEndpointsUpdated.fire(this._endpoints);
-			}).catch(err => {
-				// If an error occurs show a message so the user knows something failed but still
-				// fire the event so callers can know to update (e.g. so dashboards don't show the
-				// loading icon forever)
-				if (showErrors) {
-					vscode.window.showErrorMessage(loc.fetchEndpointsFailed(this.info.name, err));
-				}
-				this._onEndpointsUpdated.fire(this._endpoints);
-				throw err;
-			}),
-			Promise.all([
 			this._azApi.az.postgres.arcserver.list(namespace, this.azAdditionalEnvVars).then(result => {
 				newRegistrations.push(...result.stdout.map(r => {
 					return {
@@ -190,8 +165,7 @@ export class ControllerModel {
 			this._registrations = newRegistrations;
 			this.registrationsLastUpdated = new Date();
 			this._onRegistrationsUpdated.fire(this._registrations);
-			})
+		});
-		]);
 	}
 
 	public get endpoints(): azExt.DcEndpointListResult[] {

extensions/arc/src/ui/dashboards/controller/controllerDashboard.ts

@@ -19,7 +19,7 @@ export class ControllerDashboard extends Dashboard {
 	public override async showDashboard(): Promise<void> {
 		await super.showDashboard();
 		// Kick off the model refresh but don't wait on it since that's all handled with callbacks anyways
-		this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing Controller dashboard ${err}`));
+		this._controllerModel.refresh(false, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing Controller dashboard ${err}`));
 	}
 
 	protected async registerTabs(modelView: azdata.ModelView): Promise<(azdata.DashboardTab | azdata.DashboardTabGroup)[]> {

extensions/arc/src/ui/dashboards/controller/controllerDashboardOverviewPage.ts

@@ -57,7 +57,7 @@ export class ControllerDashboardOverviewPage extends DashboardPage {
 	}
 
 	protected async refresh(): Promise<void> {
-		await this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace);
+		await this._controllerModel.refresh(false, this._controllerModel.info.namespace);
 	}
 
 	public get container(): azdata.Component {

extensions/arc/src/ui/dashboards/controller/controllerUpgrades.ts

@@ -35,7 +35,7 @@ export class ControllerUpgradesPage extends DashboardPage {
 		return IconPathHelper.upgrade;
 	}
 	protected async refresh(): Promise<void> {
-		await Promise.resolve(this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace));
+		await Promise.resolve(this._controllerModel.refresh(false, this._controllerModel.info.namespace));
 		this.handleTableUpdated();
 	}
 
@@ -236,7 +236,6 @@ export class ControllerUpgradesPage extends DashboardPage {
 											this._controllerModel.info.name,
 											this._controllerModel.info.resourceGroup,
 											undefined, // Indirect mode argument - namespace
-											undefined // Indirect mode argument - usek8s
 										);
 									} else {
 										await this._azApi.az.arcdata.dc.upgrade(
@@ -244,7 +243,6 @@ export class ControllerUpgradesPage extends DashboardPage {
 											this._controllerModel.info.name,
 											undefined, // Direct mode argument - resourceGroup
 											this._controllerModel.info.namespace,
-											true
 										);
 									}
 								} else {
@@ -252,7 +250,7 @@ export class ControllerUpgradesPage extends DashboardPage {
 								}
 
 								try {
-									await this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace);
+									await this._controllerModel.refresh(false, this._controllerModel.info.namespace);
 								} catch (error) {
 									vscode.window.showErrorMessage(loc.refreshFailed(error));
 								}

extensions/arc/src/ui/dashboards/miaa/miaaBackupsPage.ts

@@ -56,7 +56,7 @@ export class MiaaBackupsPage extends DashboardPage {
 		return IconPathHelper.pitr;
 	}
 	protected async refresh(): Promise<void> {
-		await Promise.all([this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace), this._miaaModel.refresh()]);
+		await Promise.all([this._controllerModel.refresh(false, this._controllerModel.info.namespace), this._miaaModel.refresh()]);
 	}
 
 	public get container(): azdata.Component {

extensions/arc/src/ui/dashboards/miaa/miaaDashboard.ts

@@ -23,7 +23,7 @@ export class MiaaDashboard extends Dashboard {
 	public override async showDashboard(): Promise<void> {
 		await super.showDashboard();
 		// Kick off the model refreshes but don't wait on it since that's all handled with callbacks anyways
-		this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing controller model for MIAA dashboard ${err}`));
+		this._controllerModel.refresh(false, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing controller model for MIAA dashboard ${err}`));
 		this._miaaModel.refresh().catch(err => console.log(`Error refreshing MIAA model for MIAA dashboard ${err}`));
 	}
 

extensions/arc/src/ui/dashboards/miaa/miaaDashboardOverviewPage.ts

@@ -8,7 +8,7 @@ import * as azExt from 'az-ext';
 import * as azurecore from 'azurecore';
 import * as vscode from 'vscode';
 import { getDatabaseStateDisplayText, promptForInstanceDeletion } from '../../../common/utils';
-import { cssStyles, IconPathHelper, miaaTroubleshootDocsUrl } from '../../../constants';
+import { ConnectionMode, cssStyles, IconPathHelper, miaaTroubleshootDocsUrl } from '../../../constants';
 import * as loc from '../../../localizedConstants';
 import { ControllerModel } from '../../../models/controllerModel';
 import { MiaaModel } from '../../../models/miaaModel';
@@ -74,7 +74,7 @@ export class MiaaDashboardOverviewPage extends DashboardPage {
 	}
 
 	protected async refresh(): Promise<void> {
-		await Promise.all([this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace), this._miaaModel.refresh()]);
+		await Promise.all([this._controllerModel.refresh(false, this._controllerModel.info.namespace), this._miaaModel.refresh()]);
 	}
 
 	public get container(): azdata.Component {
@@ -243,7 +243,25 @@ export class MiaaDashboardOverviewPage extends DashboardPage {
 								cancellable: false
 							},
 							async (_progress, _token) => {
-								return await this._azApi.az.sql.miarc.delete(this._miaaModel.info.name, this._controllerModel.info.namespace, this._controllerModel.azAdditionalEnvVars);
+								if (this._controllerModel.info.connectionMode === ConnectionMode.direct) {
+									return await this._azApi.az.sql.miarc.delete(
+										this._miaaModel.info.name,
+										{
+											resourceGroup: this._controllerModel.info.resourceGroup,
+											namespace: undefined,
+										},
+										this._controllerModel.azAdditionalEnvVars
+									);
+								} else {
+									return await this._azApi.az.sql.miarc.delete(
+										this._miaaModel.info.name,
+										{
+											resourceGroup: undefined,
+											namespace: this._controllerModel.info.namespace,
+										},
+										this._controllerModel.azAdditionalEnvVars
+									);
+								}
 							}
 						);
 						await this._controllerModel.refreshTreeNode();

extensions/arc/src/ui/dashboards/miaa/miaaUpgradeManagementPage.ts

@@ -36,7 +36,7 @@ export class MiaaUpgradeManagementPage extends DashboardPage {
 		return IconPathHelper.upgrade;
 	}
 	protected async refresh(): Promise<void> {
-		await Promise.resolve(this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace));
+		await Promise.resolve(this._controllerModel.refresh(false, this._controllerModel.info.namespace));
 		this.handleTableUpdated();
 	}
 
@@ -160,7 +160,7 @@ export class MiaaUpgradeManagementPage extends DashboardPage {
 	private async getMiaaVersion(): Promise<string | undefined> {
 		try {
 			let miaaShowResult;
-			if (this._controllerModel.info.connectionMode === ConnectionMode.direct) {
+			if (this._controllerModel.info.connectionMode === ConnectionMode.direct || this._controllerModel.controllerConfig?.spec.settings.azure.connectionMode === ConnectionMode.direct) {
 				miaaShowResult = await this._azApi.az.sql.miarc.show(
 					this._miaaModel.info.name,
 					{
@@ -289,7 +289,7 @@ export class MiaaUpgradeManagementPage extends DashboardPage {
 								}
 
 								try {
-									await this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace);
+									await this._controllerModel.refresh(false, this._controllerModel.info.namespace);
 								} catch (error) {
 									vscode.window.showErrorMessage(loc.refreshFailed(error));
 								}

extensions/arc/src/ui/dashboards/postgres/postgresDashboard.ts

@@ -29,7 +29,7 @@ export class PostgresDashboard extends Dashboard {
 		await super.showDashboard();
 
 		// Kick off the model refresh but don't wait on it since that's all handled with callbacks anyways
-		this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing controller model for Postgres dashboard ${err}`));
+		this._controllerModel.refresh(false, this._controllerModel.info.namespace).catch(err => console.log(`Error refreshing controller model for Postgres dashboard ${err}`));
 		this._postgresModel.refresh().catch(err => console.log(`Error refreshing Postgres model for Postgres dashboard ${err}`));
 	}
 

extensions/arc/src/ui/dashboards/postgres/postgresOverviewPage.ts

@@ -295,7 +295,7 @@ export class PostgresOverviewPage extends DashboardPage {
 
 					await Promise.all([
 						this._postgresModel.refresh(),
-						this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace)
+						this._controllerModel.refresh(false, this._controllerModel.info.namespace)
 					]);
 				} catch (error) {
 					vscode.window.showErrorMessage(loc.refreshFailed(error));

extensions/arc/src/ui/dashboards/postgres/postgresPropertiesPage.ts

@@ -77,7 +77,7 @@ export class PostgresPropertiesPage extends DashboardPage {
 					this.loading!.loading = true;
 					await Promise.all([
 						this._postgresModel.refresh(),
-						this._controllerModel.refresh(false, this._controllerModel.info.resourceGroup, this._controllerModel.info.namespace)
+						this._controllerModel.refresh(false, this._controllerModel.info.namespace)
 					]);
 				} catch (error) {
 					vscode.window.showErrorMessage(loc.refreshFailed(error));

extensions/arc/src/ui/dialogs/connectControllerDialog.ts

@@ -15,6 +15,7 @@ import { AzureArcTreeDataProvider } from '../tree/azureArcTreeDataProvider';
 import { RadioOptionsGroup } from '../components/radioOptionsGroup';
 import { getCurrentClusterContext, getDefaultKubeConfigPath, getKubeConfigClusterContexts, KubeClusterContext } from '../../common/kubeUtils';
 import { FilePicker } from '../components/filePicker';
+import { ConnectionMode } from '../../constants';
 
 export type ConnectToControllerDialogModel = { controllerModel: ControllerModel };
 
@@ -196,16 +197,16 @@ export class ConnectToControllerDialog extends ControllerDialogBase {
 		const controllerModel = new ControllerModel(this.treeDataProvider, controllerInfo);
 		try {
 			// Validate that we can connect to the controller, this also populates the controllerRegistration from the connection response.
-			await controllerModel.refresh(false, this.resourceGroup, this.namespaceInputBox.value);
+			await controllerModel.refresh(false, this.namespaceInputBox.value);
 			// default info.name to the name of the controller instance if the user did not specify their own and to a pre-canned default if for some weird reason controller endpoint returned instanceName is also not a valid value
 			controllerModel.info.name = controllerModel.info.name || controllerModel.controllerConfig?.metadata.name || loc.defaultControllerName;
 			controllerModel.info.resourceGroup = <string>controllerModel.controllerConfig?.spec.settings.azure.resourceGroup;
 			controllerModel.info.connectionMode = <string>controllerModel.controllerConfig?.spec.settings.azure.connectionMode;
 			controllerModel.info.location = <string>controllerModel.controllerConfig?.spec.settings.azure.location;
 
-			if (controllerModel.info.connectionMode === 'direct') {
+			if (controllerModel.info.connectionMode === ConnectionMode.direct) {
 				const rawCustomLocation = <string>controllerModel.controllerConfig?.metadata.annotations['management.azure.com/customLocation'];
-				const exp = /\/\bsubscriptions\b\/[\S]*\/\bresourceGroups\/[\S]*\/providers\/[\S]*\/customLocations\/([\S]*)/;
+				const exp = /customlocations\/([\S]*)/;
 				controllerModel.info.customLocation = <string>exp.exec(rawCustomLocation)?.pop();
 			}
 		} catch (err) {

extensions/arc/src/ui/tree/controllerTreeNode.ts

@@ -39,12 +39,12 @@ export class ControllerTreeNode extends TreeNode {
 
 	public override async getChildren(): Promise<TreeNode[]> {
 		try {
-			await this.model.refresh(false, this.model.info.resourceGroup, this.model.info.namespace);
+			await this.model.refresh(false, this.model.info.namespace);
 			this.updateChildren(this.model.registrations);
 		} catch (err) {
 			vscode.window.showErrorMessage(loc.errorConnectingToController(err));
 			try {
-				await this.model.refresh(false, this.model.info.resourceGroup, this.model.info.namespace);
+				await this.model.refresh(false, this.model.info.namespace);
 				this.updateChildren(this.model.registrations);
 			} catch (err) {
 				if (!(err instanceof UserCancelledError)) {

extensions/azcli/src/api.ts

@@ -65,12 +65,11 @@ export function getAzApi(localAzDiscovered: Promise<IAzTool | undefined>, azTool
 					name: string,
 					resourceGroup?: string,
 					namespace?: string,
-					usek8s?: boolean,
 					additionalEnvVars?: azExt.AdditionalEnvVars
 				) => {
 					await localAzDiscovered;
 					validateAz(azToolService.localAz);
-					return azToolService.localAz!.arcdata.dc.upgrade(desiredVersion, name, resourceGroup, namespace, usek8s, additionalEnvVars);
+					return azToolService.localAz!.arcdata.dc.upgrade(desiredVersion, name, resourceGroup, namespace, additionalEnvVars);
 				}
 			}
 		},
@@ -118,10 +117,19 @@ export function getAzApi(localAzDiscovered: Promise<IAzTool | undefined>, azTool
 		},
 		sql: {
 			miarc: {
-				delete: async (name: string, namespace: string, additionalEnvVars?: azExt.AdditionalEnvVars) => {
+				delete: async (
+					name: string,
+					args: {
+						// Direct mode arguments
+						resourceGroup?: string;
+						// Indirect mode arguments
+						namespace?: string;
+					},
+					additionalEnvVars?: azExt.AdditionalEnvVars
+				) => {
 					await localAzDiscovered;
 					validateAz(azToolService.localAz);
-					return azToolService.localAz!.sql.miarc.delete(name, namespace, additionalEnvVars);
+					return azToolService.localAz!.sql.miarc.delete(name, args, additionalEnvVars);
 				},
 				list: async (
 					args: {

extensions/azcli/src/az.ts

@@ -112,13 +112,15 @@ export class AzTool implements azExt.IAzApi {
 					stderr: output.stderr
 				};
 			},
-			upgrade: (desiredVersion: string, name: string, resourceGroup?: string, namespace?: string, usek8s?: boolean, additionalEnvVars?: azExt.AdditionalEnvVars): Promise<azExt.AzOutput<void>> => {
+			upgrade: (desiredVersion: string, name: string, resourceGroup?: string, namespace?: string, additionalEnvVars?: azExt.AdditionalEnvVars): Promise<azExt.AzOutput<void>> => {
 				const argsArray = ['arcdata', 'dc', 'upgrade', '--desired-version', desiredVersion, '--name', name];
 				// Direct mode argument
 				if (resourceGroup) { argsArray.push('--resource-group', resourceGroup); }
 				// Indirect mode arguments
-				if (namespace) { argsArray.push('--k8s-namespace', namespace); }
+				if (namespace) {
-				if (usek8s) { argsArray.push('--use-k8s'); }
+					argsArray.push('--k8s-namespace', namespace);
+					argsArray.push('--use-k8s');
+				}
 				return this.executeCommand<void>(argsArray, additionalEnvVars);
 			}
 		}
@@ -175,8 +177,26 @@ export class AzTool implements azExt.IAzApi {
 
 	public sql = {
 		miarc: {
-			delete: (name: string, namespace: string, additionalEnvVars?: azExt.AdditionalEnvVars): Promise<azExt.AzOutput<void>> => {
+			delete: (
-				return this.executeCommand<void>(['sql', 'mi-arc', 'delete', '-n', name, '--k8s-namespace', namespace, '--use-k8s'], additionalEnvVars);
+				name: string,
+				args: {
+					// Direct mode arguments
+					resourceGroup?: string,
+					// Indirect mode arguments
+					namespace?: string
+					// Additional arguments
+				},
+				additionalEnvVars?: azExt.AdditionalEnvVars
+			): Promise<azExt.AzOutput<void>> => {
+				const argsArray = ['sql', 'mi-arc', 'delete', '-n', name];
+				if (args.resourceGroup) {
+					argsArray.push('--resource-group', args.resourceGroup);
+				}
+				if (args.namespace) {
+					argsArray.push('--k8s-namespace', args.namespace);
+					argsArray.push('--use-k8s');
+				}
+				return this.executeCommand<void>(argsArray, additionalEnvVars);
 			},
 			list: (
 				args: {

extensions/azcli/src/test/az.test.ts

@@ -111,7 +111,7 @@ describe('az', function () {
 			describe('mi-arc', function (): void {
 				it('delete', async function (): Promise<void> {
 					// Assume indirect mode
-					await azTool.sql.miarc.delete(name, namespace);
+					await azTool.sql.miarc.delete(name, {resourceGroup: undefined, namespace: namespace});
 					verifyExecuteCommandCalledWithArgs(['sql', 'mi-arc', 'delete', name, '--k8s-namespace', namespace, '--use-k8s']);
 				});
 				it('list', async function (): Promise<void> {

extensions/azcli/src/typings/az-ext.d.ts

@@ -534,7 +534,7 @@ declare module 'az-ext' {
 					show(namespace?: string, additionalEnvVars?: AdditionalEnvVars): Promise<AzOutput<DcConfigShowResult>>
 				},
 				listUpgrades(namespace: string, usek8s?: boolean, additionalEnvVars?: AdditionalEnvVars): Promise<AzOutput<DcListUpgradesResult>>,
-				upgrade(desiredVersion: string, name: string, resourceGroup?: string, namespace?: string, usek8s?: boolean, additionalEnvVars?: AdditionalEnvVars): Promise<AzOutput<void>>,
+				upgrade(desiredVersion: string, name: string, resourceGroup?: string, namespace?: string, additionalEnvVars?: AdditionalEnvVars): Promise<AzOutput<void>>,
 			}
 		},
 		postgres: {
@@ -566,7 +566,17 @@ declare module 'az-ext' {
 		},
 		sql: {
 			miarc: {
-				delete(name: string, namespace?: string, additionalEnvVars?: AdditionalEnvVars): Promise<AzOutput<void>>,
+				delete(
+					name: string,
+					args: {
+						// Direct mode arguments
+						resourceGroup?: string,
+						// Indirect mode arguments
+						namespace?: string
+						},
+						// Additional arguments
+						additionalEnvVars?: AdditionalEnvVars
+				): Promise<AzOutput<void>>,
 				list(
 					args: {
 					// Direct mode arguments