-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[PostgreSQL] BREAKING CHANGE: az postgres flexible-server create/update: Remove deprecated --cluster-option and update validation logic
#33244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
58af789
ef50eb6
add7cac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,9 +99,9 @@ def retention_validator(ns): | |
|
|
||
|
|
||
| def db_renaming_cluster_validator(ns): | ||
| if ns.database_name is not None and ns.create_cluster.lower() != 'elasticcluster': | ||
| if ns.database_name is not None and ns.cluster_size is None: | ||
| raise ArgumentUsageError('The --database-name argument can only be used ' | ||
| 'when --cluster-option is set to "ElasticCluster".') | ||
| 'when --node-count is present, as it only applies to elastic clusters.') | ||
|
|
||
|
|
||
| # Validates if a subnet id or name have been given by the user. If subnet id is given, vnet-name should not be provided. | ||
|
|
@@ -150,7 +150,7 @@ def pg_arguments_validator(db_context, location, tier, sku_name, storage_gb, ser | |
| public_access=None, version=None, instance=None, geo_redundant_backup=None, | ||
| byok_identity=None, byok_key=None, backup_byok_identity=None, backup_byok_key=None, | ||
| auto_grow=None, performance_tier=None, | ||
| storage_type=None, iops=None, throughput=None, create_cluster=None, cluster_size=None, | ||
| storage_type=None, iops=None, throughput=None, cluster_size=None, | ||
| password_auth=None, microsoft_entra_auth=None, | ||
| admin_name=None, admin_id=None, admin_type=None): | ||
| validate_server_name(db_context, server_name, 'Microsoft.DBforPostgreSQL/flexibleServers') | ||
|
|
@@ -169,7 +169,7 @@ def pg_arguments_validator(db_context, location, tier, sku_name, storage_gb, ser | |
| sku_info = {k.lower(): v for k, v in sku_info.items()} | ||
| single_az = list_location_capability_info['single_az'] | ||
| geo_backup_supported = list_location_capability_info['geo_backup_supported'] | ||
| _cluster_validator(create_cluster, cluster_size, auto_grow, version, instance) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is auto_grow and version removed from the validation? |
||
| _cluster_validator(cluster_size, instance) | ||
| _network_arg_validator(subnet, public_access) | ||
| _pg_tier_validator(tier, sku_info) # need to be validated first | ||
| if tier is None and instance is not None: | ||
|
|
@@ -198,16 +198,8 @@ def pg_arguments_validator(db_context, location, tier, sku_name, storage_gb, ser | |
| admin_name, admin_id, admin_type, instance) | ||
|
|
||
|
|
||
| def _cluster_validator(create_cluster, cluster_size, auto_grow, version, instance): | ||
| if (create_cluster and create_cluster.lower() == 'elasticcluster') or \ | ||
| (instance and instance.cluster and instance.cluster.cluster_size > 0): | ||
| if instance is None and version != '17': | ||
| raise ValidationError('Elastic cluster is only supported for PostgreSQL version 17.') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is supported for version 17 and up. If we get sensible messages from the control plane then we can indeed drop CLI validation. |
||
|
|
||
| if auto_grow and auto_grow.lower() != 'disabled': | ||
| raise ValidationError('Storage auto-grow is not supported for elastic cluster.') | ||
|
|
||
| if cluster_size and instance and not instance.cluster: | ||
| def _cluster_validator(cluster_size, instance): | ||
| if cluster_size is not None and instance and not instance.cluster: | ||
| raise ValidationError('Node count can only be specified for an elastic cluster.') | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is passing database_name related to removing the deprecated cluster option? Should this be a separate PR and is it fixing an existing bug/exposing new functionality?