Edit Postgres Engine Settings Per Role (#15481)

* Enables being able to view and edit Coordinator node scheduling params (#15114)

* Trying to save per role settings

* Updated spec

* Cleaning up

* Removed unneccessary code and comments

* Added separate type for { w?: string, c?: string}, PR fixes

* Added methods to refresh mr,ml,cr,cl versus per role

* Fixed spec

* Put back optional properties, removed passing empty string to reset scheduling params

* Spacing

* vBump arc

* Included roles in fake show output for testing (#15196)

* Update arc specs (#15225)

* Update azdata Arc specs to match April azdata

* vcores -> cpu

* fix spacing

* Consolidate types and update storage volumes

* Fix compile

* Fix spec

* include coordinator

* Adding args

* Query call success

* Check for success in query

* List full coordinator params

* Change name

* Update unit test for engine settings

* Pr changes

* Fix query

Co-authored-by: Charles Gagnon <chgagnon@microsoft.com>

extensions/arc/src/localizedConstants.ts

@@ -23,7 +23,6 @@ export const properties = localize('arc.properties', "Properties");
 export const settings = localize('arc.settings', "Settings");
 export const security = localize('arc.security', "Security");
 export const computeAndStorage = localize('arc.computeAndStorage', "Compute + Storage");
-export const nodeParameters = localize('arc.nodeParameters', "Node Parameters");
 export const coordinatorNodeParameters = localize('arc.coordinatorNodeParameters', "Coordinator Node Parameters");
 export const workerNodeParameters = localize('arc.workerNodeParameters', "Worker Node Parameters");
 export const compute = localize('arc.compute', "Compute");
@@ -146,7 +145,6 @@ export const databaseName = localize('arc.databaseName', "Database name");
 export const enterNewPassword = localize('arc.enterNewPassword', "Enter a new password");
 export const confirmNewPassword = localize('arc.confirmNewPassword', "Confirm the new password");
 export const learnAboutPostgresClients = localize('arc.learnAboutPostgresClients', "Learn more about Azure PostgreSQL Hyperscale client interfaces");
-export const nodeParametersDescription = localize('arc.nodeParametersDescription', " These server parameters of the Coordinator node and the Worker nodes can be set to custom (non-default) values. Search to find parameters.");
 export const coordinatorNodeParametersDescription = localize('arc.coordinatorNodeParametersDescription', " These server parameters of the Coordinator node can be set to custom (non-default) values. Search to find parameters.");
 export const workerNodesParametersDescription = localize('arc.workerNodesParametersDescription', " These server parameters of the Worker nodes can be set to custom (non-default) values. Search to find parameters.");
 export const learnAboutNodeParameters = localize('arc.learnAboutNodeParameters', "Learn more about database engine settings for Azure Arc enabled PostgreSQL Hyperscale");
@@ -181,6 +179,7 @@ export const condition = localize('arc.condition', "Condition");
 export const details = localize('arc.details', "Details");
 export const lastTransition = localize('arc.lastTransition', "Last transition");
 export const noExternalEndpoint = localize('arc.noExternalEndpoint', "No External Endpoint has been configured so this information isn't available.");
+export const noWorkerPods = localize('arc.noWorkerPods', "No worker pods in this configuration.");
 export const podsReady = localize('arc.podsReady', "pods ready");
 export const podsPresent = localize('arc.podsPresent', "Pods Present");
 export const podsUsedDescription = localize('arc.podsUsedDescription', "Select a pod in the dropdown below for detailed health information.");

extensions/arc/src/models/postgresModel.ts

@@ -155,36 +155,18 @@ export class PostgresModel extends ResourceModel {
 			const provider = azdata.dataprotocol.getProvider<azdata.QueryProvider>(this._connectionProfile!.providerName, azdata.DataProviderType.QueryProvider);
 			const ownerUri = await azdata.connection.getUriForConnection(this._activeConnectionId);
 
-			const engineSettings = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings');
-			if (!engineSettings) {
-				throw new Error('Could not fetch engine settings');
-			}
-
 			const skippedEngineSettings: String[] = [
 				'archive_command', 'archive_timeout', 'log_directory', 'log_file_mode', 'log_filename', 'restore_command',
 				'shared_preload_libraries', 'synchronous_commit', 'ssl', 'unix_socket_permissions', 'wal_level'
 			];
 
-			this.workerNodesEngineSettings = [];
-
-			engineSettings.rows.forEach(row => {
-				let rowValues = row.map(c => c.displayValue);
-				let name = rowValues.shift();
-				if (!skippedEngineSettings.includes(name!)) {
-					let result: EngineSettingsModel = {
-						parameterName: name,
-						value: rowValues.shift(),
-						description: rowValues.shift(),
-						min: rowValues.shift(),
-						max: rowValues.shift(),
-						options: rowValues.shift(),
-						type: rowValues.shift()
-					};
-
-					this.workerNodesEngineSettings.push(result);
-				}
-			});
+			await this.createCoordinatorEngineSettings(provider, ownerUri, skippedEngineSettings);
 
+			const scale = this._config?.spec.scale;
+			const nodes = (scale?.workers ?? scale?.shards ?? 0);
+			if (nodes !== 0) {
+				await this.createWorkerEngineSettings(provider, ownerUri, skippedEngineSettings);
+			}
 
 			this.engineSettingsLastUpdated = new Date();
 			this._engineSettingsPromise.resolve();
@@ -196,6 +178,64 @@ export class PostgresModel extends ResourceModel {
 		}
 	}
 
+	private async createCoordinatorEngineSettings(provider: azdata.QueryProvider, ownerUri: string, skip: String[]): Promise<void> {
+		const engineSettingsCoordinator = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings');
+
+		this.coordinatorNodeEngineSettings = [];
+		engineSettingsCoordinator.rows.forEach(row => {
+			let rowValues = row.map(c => c.displayValue);
+			let name = rowValues.shift();
+			if (!skip.includes(name!)) {
+				let result: EngineSettingsModel = {
+					parameterName: name,
+					value: rowValues.shift(),
+					description: rowValues.shift(),
+					min: rowValues.shift(),
+					max: rowValues.shift(),
+					options: rowValues.shift(),
+					type: rowValues.shift()
+				};
+
+				this.coordinatorNodeEngineSettings.push(result);
+			}
+		});
+
+	}
+
+	private async createWorkerEngineSettings(provider: azdata.QueryProvider, ownerUri: string, skip: String[]): Promise<void> {
+
+		const engineSettingsWorker = await provider.runQueryAndReturn(ownerUri,
+			`with settings as (select nodename, success, result from run_command_on_workers('select json_agg(pg_settings) from pg_settings') order by success desc, nodename asc)
+			select * from settings limit case when exists(select 1 from settings where success) then 1 end`);
+
+		if (engineSettingsWorker.rows[0][1].displayValue === 'False') {
+			let errorString = engineSettingsWorker.rows.map(row => row[2].displayValue);
+			throw new Error(errorString.join('\n'));
+		}
+
+		let engineSettingsWorkerJSON = JSON.parse(engineSettingsWorker.rows[0][2].displayValue);
+		this.workerNodesEngineSettings = [];
+
+		for (let i = 0; i < engineSettingsWorkerJSON.length; i++) {
+			let rowValues = engineSettingsWorkerJSON[i];
+			let name = rowValues.name;
+			if (!skip.includes(name!)) {
+				let result: EngineSettingsModel = {
+					parameterName: name,
+					value: rowValues.setting,
+					description: rowValues.short_desc,
+					min: rowValues.min_val,
+					max: rowValues.max_val,
+					options: rowValues.enumvals,
+					type: rowValues.vartype
+				};
+
+				this.workerNodesEngineSettings.push(result);
+			}
+		}
+
+	}
+
 	protected createConnectionProfile(): azdata.IConnectionProfile {
 		const ipAndPort = parseIpAndPort(this.config?.status.primaryEndpoint || '');
 		return {

extensions/arc/src/test/mocks/fakeAzdataApi.ts

@@ -33,6 +33,7 @@ export class FakeAzdataApi implements azdataExt.IAzdataApi {
 						adminPassword?: boolean,
 						coresLimit?: string,
 						coresRequest?: string,
+						coordinatorEngineSettings?: string,
 						engineSettings?: string,
 						extensions?: string,
 						memoryLimit?: string,
@@ -40,6 +41,7 @@ export class FakeAzdataApi implements azdataExt.IAzdataApi {
 						noWait?: boolean,
 						port?: number,
 						replaceEngineSettings?: boolean,
+						workerEngineSettings?: string,
 						workers?: number
 					},
 					_additionalEnvVars?: azdataExt.AdditionalEnvVars

extensions/arc/src/test/models/postgresModel.test.ts

@@ -19,6 +19,11 @@ import { AzureArcTreeDataProvider } from '../../ui/tree/azureArcTreeDataProvider
 import { FakeControllerModel } from '../mocks/fakeControllerModel';
 import { FakeAzdataApi } from '../mocks/fakeAzdataApi';
 
+export const FakeStorageVolume:  azdataExt.StorageVolume[] = [{
+	className: '',
+	size: ''
+}];
+
 export const FakePostgresServerShowOutput: azdataExt.AzdataOutput<azdataExt.PostgresServerShowResult> = {
 	logs: [],
 	stdout: [],
@@ -39,7 +44,11 @@ export const FakePostgresServerShowOutput: azdataExt.AzdataOutput<azdataExt.Post
 			engine: {
 				extensions: [{ name: '' }],
 				settings: {
-					default: { ['']: '' }
+					default: { ['']: '' },
+					roles: {
+						coordinator: { ['']: '' },
+						worker: { ['']: '' }
+					}
 				},
 				version: ''
 			},
@@ -553,7 +562,7 @@ describe('PostgresModel', function (): void {
 			sinon.stub(azdata.dataprotocol, 'getProvider').returns(providerMock.object);
 
 			await postgresModel.getEngineSettings();
-			should(postgresModel.workerNodesEngineSettings.pop()).be.match(engineSettingsModelCompare);
+			should(postgresModel.coordinatorNodeEngineSettings.pop()).be.match(engineSettingsModelCompare);
 		});
 
 	});

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

@@ -35,33 +35,28 @@ export class PostgresCoordinatorNodeParametersPage extends PostgresParametersPag
 		return this._postgresModel.coordinatorNodeEngineSettings;
 	}
 
-	protected async saveParameterEdits(): Promise<void> {
-		/* TODO add correct azdata call for editing coordinator parameters
-			await this._azdataApi.azdata.arc.postgres.server.edit(
-				this._postgresModel.info.name,
-				{ engineSettings: engineSettings.toString() },
-				this._postgresModel.controllerModel.azdataAdditionalEnvVars,
-				session);
-		*/
+	protected async saveParameterEdits(engineSettings: string): Promise<void> {
+		await this._azdataApi.azdata.arc.postgres.server.edit(
+			this._postgresModel.info.name,
+			{ coordinatorEngineSettings: engineSettings },
+			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
+			this._postgresModel.controllerModel.controllerContext);
+
 	}
 
 	protected async resetAllParameters(): Promise<void> {
-		/* TODO add correct azdata call for editing coordinator parameters
-			await this._azdataApi.azdata.arc.postgres.server.edit(
-				this._postgresModel.info.name,
-				{ engineSettings: `''`, replaceEngineSettings: true },
-				this._postgresModel.controllerModel.azdataAdditionalEnvVars,
-				session);
-		*/
+		await this._azdataApi.azdata.arc.postgres.server.edit(
+			this._postgresModel.info.name,
+			{ coordinatorEngineSettings: `''`, replaceEngineSettings: true },
+			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
+			this._postgresModel.controllerModel.controllerContext);
 	}
 
-	protected async resetParameter(): Promise<void> {
-		/* TODO add correct azdata call for editing coordinator parameters
-			await this._azdataApi.azdata.arc.postgres.server.edit(
-				this._postgresModel.info.name,
-				{ engineSettings: parameterName + '=' },
-				this._postgresModel.controllerModel.azdataAdditionalEnvVars,
-				session);
-		*/
+	protected async resetParameter(parameterName: string): Promise<void> {
+		await this._azdataApi.azdata.arc.postgres.server.edit(
+			this._postgresModel.info.name,
+			{ coordinatorEngineSettings: parameterName + '=' },
+			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
+			this._postgresModel.controllerModel.controllerContext);
 	}
 }

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

@@ -17,6 +17,7 @@ import { PostgresComputeAndStoragePage } from './postgresComputeAndStoragePage';
 import { PostgresWorkerNodeParametersPage } from './postgresWorkerNodeParametersPage';
 import { PostgresPropertiesPage } from './postgresPropertiesPage';
 import { PostgresResourceHealthPage } from './postgresResourceHealthPage';
+import { PostgresCoordinatorNodeParametersPage } from './postgresCoordinatorNodeParametersPage';
 
 export class PostgresDashboard extends Dashboard {
 	constructor(private _context: vscode.ExtensionContext, private _controllerModel: ControllerModel, private _postgresModel: PostgresModel) {
@@ -36,8 +37,7 @@ export class PostgresDashboard extends Dashboard {
 		const connectionStringsPage = new PostgresConnectionStringsPage(modelView, this.dashboard, this._postgresModel);
 		const computeAndStoragePage = new PostgresComputeAndStoragePage(modelView, this.dashboard, this._postgresModel);
 		const propertiesPage = new PostgresPropertiesPage(modelView, this.dashboard, this._controllerModel, this._postgresModel);
-		// TODO Add dashboard once backend is able to be connected for per role server parameter edits.
-		// const coordinatorNodeParametersPage = new PostgresCoordinatorNodeParametersPage(modelView, this._postgresModel);
+		const coordinatorNodeParametersPage = new PostgresCoordinatorNodeParametersPage(modelView, this.dashboard, this._postgresModel);
 		const workerNodeParametersPage = new PostgresWorkerNodeParametersPage(modelView, this.dashboard, this._postgresModel);
 		const diagnoseAndSolveProblemsPage = new PostgresDiagnoseAndSolveProblemsPage(modelView, this.dashboard, this._context, this._controllerModel, this._postgresModel);
 		const supportRequestPage = new PostgresSupportRequestPage(modelView, this.dashboard, this._controllerModel, this._postgresModel);
@@ -51,6 +51,7 @@ export class PostgresDashboard extends Dashboard {
 					propertiesPage.tab,
 					connectionStringsPage.tab,
 					computeAndStoragePage.tab,
+					coordinatorNodeParametersPage.tab,
 					workerNodeParametersPage.tab
 				]
 			},

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

@@ -269,6 +269,13 @@ export abstract class PostgresParametersPage extends DashboardPage {
 
 		this.disposables.push(
 			this.connectToServerButton.onDidClick(async () => {
+				let scale = this._postgresModel.config?.spec.scale;
+				let nodes = (scale?.workers ?? scale?.shards ?? 0);
+				if (this.title === loc.workerNodeParameters && nodes === 0) {
+					vscode.window.showInformationMessage(loc.noWorkerPods);
+					return;
+				}
+
 				this.connectToServerButton!.enabled = false;
 				if (!vscode.extensions.getExtension(loc.postgresExtension)) {
 					const response = await vscode.window.showErrorMessage(loc.missingExtension('PostgreSQL'), loc.yes, loc.no);
@@ -437,11 +444,13 @@ export abstract class PostgresParametersPage extends DashboardPage {
 		let valueComponent: azdata.Component;
 		if (engineSetting.type === 'enum') {
 			// If type is enum, component should be drop down menu
-			let options = engineSetting.options?.slice(1, -1).split(',');
 			let values: string[] = [];
-			options!.forEach(option => {
-				values.push(option.slice(option.indexOf('"') + 1, -1));
-			});
+			if (typeof engineSetting.options === 'string') {
+				let options = engineSetting.options?.slice(1, -1).split(',');
+				values = options.map(option => option.slice(option.indexOf('"') + 1, -1));
+			} else if (engineSetting.options) {
+				values = engineSetting.options;
+			}
 
 			let valueBox = this.modelView.modelBuilder.dropDown().withProps({
 				values: values,

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

@@ -16,13 +16,11 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage {
 	}
 
 	protected get title(): string {
-		// TODO update to loc.workerNodeParameters
-		return loc.nodeParameters;
+		return loc.workerNodeParameters;
 	}
 
 	protected get id(): string {
-		// TODO update to 'postgres-worker-node-parameters'
-		return 'postgres-nodes-parameters';
+		return 'postgres-worker-node-parameters';
 	}
 
 	protected get icon(): { dark: string; light: string; } {
@@ -30,8 +28,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage {
 	}
 
 	protected get description(): string {
-		// TODO update to loc.workerNodesParametersDescription
-		return loc.nodeParametersDescription;
+		return loc.workerNodesParametersDescription;
 	}
 
 
@@ -42,7 +39,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage {
 	protected async saveParameterEdits(engineSettings: string): Promise<void> {
 		await this._azdataApi.azdata.arc.postgres.server.edit(
 			this._postgresModel.info.name,
-			{ engineSettings: engineSettings },
+			{ workerEngineSettings: engineSettings },
 			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
 			this._postgresModel.controllerModel.controllerContext);
 	}
@@ -50,7 +47,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage {
 	protected async resetAllParameters(): Promise<void> {
 		await this._azdataApi.azdata.arc.postgres.server.edit(
 			this._postgresModel.info.name,
-			{ engineSettings: `''`, replaceEngineSettings: true },
+			{ workerEngineSettings: `''`, replaceEngineSettings: true },
 			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
 			this._postgresModel.controllerModel.controllerContext);
 	}
@@ -58,7 +55,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage {
 	protected async resetParameter(parameterName: string): Promise<void> {
 		await this._azdataApi.azdata.arc.postgres.server.edit(
 			this._postgresModel.info.name,
-			{ engineSettings: parameterName + '=' },
+			{ workerEngineSettings: parameterName + '=' },
 			this._postgresModel.controllerModel.azdataAdditionalEnvVars,
 			this._postgresModel.controllerModel.controllerContext);
 	}