Conversation
️✔️AzureCLI-FullTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
️✔️AzureCLI-BreakingChangeTest
|
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull request overview
This PR aims to enable fabric mirroring on PostgreSQL version 17+ servers with high availability (HA) enabled, lifting the previous restriction where HA and fabric mirroring were mutually exclusive.
- Updated version checks in
flexible_server_update_custom_functo skip fabric mirroring validation for PG 17+ - Modified fabric mirroring start, stop, and update-databases functions to allow operations on HA-enabled servers for PG 17+
- Updated comments to reflect Ignite 2025 support for fabric mirroring with HA on PG 17
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| server = flexible_servers_client.get(resource_group_name, server_name) | ||
|
|
||
| if server.high_availability.mode != "Disabled": | ||
| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: |
There was a problem hiding this comment.
Same logic error as in the other fabric mirroring functions. The condition server.version not in ["17"] will incorrectly block database updates for all versions except 17 when HA is enabled.
Use the same fix as recommended for lines 1588 and 1618:
if server.high_availability.mode != "Disabled" and int(server.version) < 17:| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: | |
| if server.high_availability.mode != "Disabled" and int(server.version) < 17: |
|
|
||
| # PG 11 and 12 will never receive fabric mirroring support. Skip this check for servers of these versions | ||
| if high_availability.lower() != "disabled" and str(instance.version) not in ["11", "12"]: | ||
| # PG 11 and 12 will never receive fabric mirroring support. Ignite 2025 Fabric mirroring supported on 17. Skip this check for servers of these versions |
There was a problem hiding this comment.
The comment is misleading. It states "Ignite 2025 Fabric mirroring supported on 17" but the logic skips the check for version 17, which is the opposite of what the comment suggests. The comment should clarify that the check is being skipped for versions where fabric mirroring + HA is supported (17+).
| # PG 11 and 12 will never receive fabric mirroring support. Ignite 2025 Fabric mirroring supported on 17. Skip this check for servers of these versions | |
| # Skip this check for versions where fabric mirroring and high availability are supported together (17 and above), | |
| # and for PG 11 and 12, which will never receive fabric mirroring support. For all other versions, prevent enabling HA when fabric mirroring is active. |
| # PG 11 and 12 will never receive fabric mirroring support. Ignite 2025 Fabric mirroring supported on 17. Skip this check for servers of these versions | ||
| if high_availability.lower() != "disabled" and str(instance.version) not in ["11", "12", "17", "18"]: |
There was a problem hiding this comment.
The logic here is inverted. The condition str(instance.version) not in ["11", "12", "17"] means that for version 17, the fabric mirroring check will be skipped. This allows HA to be enabled without checking if fabric mirroring is active, which could lead to an invalid state.
The correct logic should be: if the version is 17 or higher, allow both HA and fabric mirroring to coexist. For versions 13-16, prevent enabling HA if fabric mirroring is active.
Consider changing to:
# PG 11 and 12 will never receive fabric mirroring support. Ignite 2025: Fabric mirroring supported with HA on 17+
if high_availability.lower() != "disabled" and str(instance.version) in ["13", "14", "15", "16"]:| # PG 11 and 12 will never receive fabric mirroring support. Ignite 2025 Fabric mirroring supported on 17. Skip this check for servers of these versions | |
| if high_availability.lower() != "disabled" and str(instance.version) not in ["11", "12", "17", "18"]: | |
| # PG 11 and 12 will never receive fabric mirroring support. Ignite 2025: Fabric mirroring supported with HA on 17+ | |
| if high_availability.lower() != "disabled" and str(instance.version) in ["13", "14", "15", "16"]: |
| server = flexible_servers_client.get(resource_group_name, server_name) | ||
|
|
||
| if server.high_availability.mode != "Disabled": | ||
| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: |
There was a problem hiding this comment.
The logic is incorrect. The condition server.version not in ["17"] means:
- For version 17: condition is
False, error is NOT raised ✓ (correct) - For versions 11-16: condition is
True, error IS raised ✗ (incorrect for intended behavior)
This prevents fabric mirroring on all non-17 versions with HA, including future versions like 18, 19, etc.
The correct approach should check if the version is less than 17:
if server.high_availability.mode != "Disabled" and int(server.version) < 17:Alternatively, if you want to be explicit about supported versions:
if server.high_availability.mode != "Disabled" and server.version in ["11", "12", "13", "14", "15", "16"]:| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: | |
| if server.high_availability.mode != "Disabled" and int(server.version) < 17: |
| server = flexible_servers_client.get(resource_group_name, server_name) | ||
|
|
||
| if server.high_availability.mode != "Disabled": | ||
| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: |
There was a problem hiding this comment.
Same logic error as in flexible_server_fabric_mirroring_start. The condition server.version not in ["17"] will block fabric mirroring stop operations for all versions except 17 when HA is enabled, including future versions.
Use the same fix as recommended for line 1588:
if server.high_availability.mode != "Disabled" and int(server.version) < 17:| if server.high_availability.mode != "Disabled" and server.version not in ["17", "18"]: | |
| if server.high_availability.mode != "Disabled" and int(server.version) < 17: |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
…high availability enabled servers to start fabric mirroring if PG version 17+ (Azure#32468)
Related command
az postgres flexible-server updateaz postgres flexible-server update/fabric-mirroringDescription
Starting with Ignite, Fabric mirroring shall be supported on PG17+ servers with HA setup as well.
Testing Guide
Manual
az postgres flexible-server fabric-mirroring start -g nascrunner25 -s nasc-new-ha-919 --database-names postgres --yes
Enabling system assigned managed identity on the server.
Updating necessary server parameters.
{
"allowedValues": ".*",
"dataType": "String",
"defaultValue": "",
"description": "Specifies the list of databases for which to enable mirroring.",
"documentationLink": "https://go.microsoft.com/fwlink/?linkid=2285682",
"id": "/subscriptions/ac0271d6-426b-4b0d-b88d-0d0e4bd693ae/resourceGroups/nascrunner25/providers/Microsoft.DBforPostgreSQL/flexibleServers/nasc-new-ha-919/configurations/azure.mirror_databases",
"isConfigPendingRestart": false,
"isDynamicConfig": true,
"isReadOnly": false,
"name": "azure.mirror_databases",
"resourceGroup": "nascrunner25",
"source": "user-override",
"systemData": null,
"type": "Microsoft.DBforPostgreSQL/flexibleServers/configurations",
"unit": null,
"value": "postgres"
}
History Notes
[RDBMS]
az postgres flexible-server update/fabric-mirroring: Allow high availability enabled servers to start fabric mirroring if PG version 17+