[PostgreSQL] az postgres flexible-server: Improved validation logic and style of error messages

src/azure-cli/azure/cli/command_modules/postgresql/tests/latest/test_postgres_flexible_commands.py

@@ -39,17 +39,17 @@ def _test_flexible_server_mgmt(self, resource_group):
         zonal_resiliency_value = 'Enabled'
         ha_value = 'ZoneRedundant'
 
-        # list skus
+        # List SKUs
         self.cmd('postgres flexible-server list-skus -l {}'.format(location),
                  checks=[JMESPathCheck('type(@)', 'array')])
 
-        # create server
+        # Create server
         self.cmd('postgres flexible-server create -g {} -n {} --backup-retention {} --sku-name {} --tier {} \
                   --storage-size {} -u {} --version {} --tags keys=3 --zonal-resiliency {} --location {}\
                   --public-access None'.format(resource_group, server_name, backup_retention,
                                                sku_name, tier, storage_size, 'dbadmin', version, zonal_resiliency_value, location))
 
-        # show server
+        # Validate several properties of the server
         basic_info = self.cmd('postgres flexible-server show -g {} -n {}'.format(resource_group, server_name)).get_output_in_json()
         self.assertEqual(basic_info['name'], server_name)
         self.assertEqual(str(basic_info['location']).replace(' ', '').lower(), location)
@@ -61,11 +61,11 @@ def _test_flexible_server_mgmt(self, resource_group):
         self.assertEqual(basic_info['backup']['backupRetentionDays'], backup_retention)
         self.assertEqual(basic_info['highAvailability']['mode'], ha_value)
 
-        # list servers
+        # List servers
         self.cmd('postgres flexible-server list -g {}'.format(resource_group),
                  checks=[JMESPathCheck('type(@)', 'array')])
 
-        # show connection string
+        # Show connection string
         connection_string = self.cmd('postgres flexible-server show-connection-string -s {}'
                                      .format(server_name)).get_output_in_json()
         self.assertIn('jdbc', connection_string['connectionStrings'])
@@ -74,11 +74,11 @@ def _test_flexible_server_mgmt(self, resource_group):
         self.assertIn('python', connection_string['connectionStrings'])
         self.assertIn('ado.net', connection_string['connectionStrings'])
 
-        # update password
+        # Update password
         self.cmd('postgres flexible-server update -g {} -n {} -p randompw321##@!'
                  .format(resource_group, server_name))
 
-        # update compute and storage
+        # Update compute and storage
         self.cmd('postgres flexible-server update -g {} -n {} --storage-size 256 --yes'
                  .format(resource_group, server_name),
                  checks=[JMESPathCheck('storage.storageSizeGb', 256 )])
@@ -105,25 +105,25 @@ def _test_flexible_server_mgmt(self, resource_group):
                  checks=[JMESPathCheck('sku.tier', tier),
                          JMESPathCheck('sku.name', sku_name)])
 
-        # update backup retention
+        # Update backup retention
         self.cmd('postgres flexible-server update -g {} -n {} --backup-retention {}'
                  .format(resource_group, server_name, backup_retention + 10),
                  checks=[JMESPathCheck('backup.backupRetentionDays', backup_retention + 10)])
 
-        # update maintenance window
+        # Update maintenance window
         maintainence_window = 'SUN'
         maintainence_window_value = 0   # Sunday is defined as 0
 
         self.cmd('postgres flexible-server update -g {} -n {} --maintenance-window {}'
                  .format(resource_group, server_name, maintainence_window),
                  checks=[JMESPathCheck('maintenanceWindow.dayOfWeek', maintainence_window_value)])
 
-        # update tags
+        # Update tags
         self.cmd('postgres flexible-server update -g {} -n {} --tags keys=3'
                  .format(resource_group, server_name),
                  checks=[JMESPathCheck('tags.keys', '3')])
 
-        # restart, stop, start server
+        # Restart, stop, start server
         self.cmd('postgres flexible-server restart -g {} -n {}'
                  .format(resource_group, server_name), checks=NoneCheck())
 
@@ -133,7 +133,7 @@ def _test_flexible_server_mgmt(self, resource_group):
         self.cmd('postgres flexible-server start -g {} -n {}'
                  .format(resource_group, server_name), checks=NoneCheck())
 
-        # expect failures
+        # Negative tests
         replica_1_name = self.create_random_name(SERVER_NAME_PREFIX, SERVER_NAME_MAX_LENGTH)
 
         self.cmd('postgres flexible-server replica create -g "" --replica-name {} --source-server {}'.format(
@@ -149,11 +149,11 @@ def _test_flexible_server_mgmt(self, resource_group):
         self.cmd('postgres flexible-server update -g \'\' -n {} -p randompw321##@!'
                  .format(server_name), expect_failure=True)
 
-        # delete
+        # Delete server
         self.cmd('postgres flexible-server delete -g {} -n {} --yes'.format(resource_group, server_name), checks=NoneCheck())
         os.environ.get(ENV_LIVE_TEST, False) and sleep(300)
 
-        # revive dropped server
+        # Revive dropped server
         revived_server_name = self.create_random_name(SERVER_NAME_PREFIX, SERVER_NAME_MAX_LENGTH)
         source_server_id = basic_info['id']
         revive_dropped_server = self.cmd('postgres flexible-server revive-dropped -g {} -n {} --source-server {} --location {}'.format(
@@ -187,57 +187,72 @@ def _test_mgmt_create_validator(self, resource_group):
         invalid_backup_retention = 40
         ha_value = 'ZoneRedundant'
 
-        # Create
+        # Create server with invalid server name
         self.cmd('postgres flexible-server create -g {} -n Wrongserver.Name -l {}'.format(
                 resource_group, location),
                 expect_failure=True)
 
+        # Create server with invalid tier
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --tier {}'.format(
                  resource_group, server_name, location, invalid_tier),
                  expect_failure=True)
 
+        # Create server with invalid version
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --version {}'.format(
                  resource_group, server_name, location, invalid_version),
                  expect_failure=True)
 
+        # Create server with invalid sku name
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --tier {} --sku-name {}'.format(
                  resource_group, server_name, location, valid_tier, invalid_sku_name),
                  expect_failure=True)
 
+        # Create server with invalid backup retention days
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --backup-retention {}'.format(
                  resource_group, server_name, location, invalid_backup_retention),
                  expect_failure=True)
 
+        # Create server with zone redundant high availability in a location that does not support it, because it's a single zone location
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --high-availability {} '.format(
                  resource_group, server_name, location, ha_value),
                  expect_failure=True)
 
-        # high availability validator
+        # Create server with zone redundant high availability with a tier that does not support it
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --tier Burstable --sku-name Standard_B1ms --high-availability {}'.format(
                  resource_group, server_name, location, ha_value),
                  expect_failure=True)
 
+        # Create server with zone redundant high availability in a location that does not support it, because it's a single zone location
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --tier GeneralPurpose --sku-name Standard_D4ds_v4 --high-availability {}'.format(
-                 resource_group, server_name, location, ha_value), # single availability zone location
+                 resource_group, server_name, location, ha_value),
                  expect_failure=True)
 
+        # Create server with zone redundant high availability and forcing same zone for primary and standby
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --tier GeneralPurpose --sku-name Standard_D2ads_v5 --high-availability {} --zone 1 --standby-zone 1'.format(
-                 resource_group, server_name, location, ha_value), # single availability zone location
+                 resource_group, server_name, location, ha_value),
                  expect_failure=True)
 
-        # Network
+        # Create server with public access and subnet id at the same time
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --vnet testvnet --subnet testsubnet --public-access All'.format(
                  resource_group, server_name, location),
                  expect_failure=True)
 
+        # Create server with subnet id that does not exist
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --subnet testsubnet'.format(
                  resource_group, server_name, location),
                  expect_failure=True)
 
+        # Create server with invalid public access value (end-ip is not a valid IPv4 address)
         self.cmd('postgres flexible-server create -g {} -n {} -l {} --public-access 12.0.0.0-10.0.0.0.0'.format(
                  resource_group, server_name, location),
                  expect_failure=True)
 
+        # Create server with invalid public access value (start-ip greater than end-ip)
+        self.cmd('postgres flexible-server create -g {} -n {} -l {} --public-access 12.0.0.0-10.0.0.0'.format(
+                 resource_group, server_name, location),
+                 expect_failure=True)
+
+        # Create server with invalid storage size (in Premium_LRS, valid storage sizes are 32, 64, 128, 256, 512, 1024, 2048, 4095, 4096, 8192, 16384, 32768, 65536)
         invalid_storage_size = 60
         self.cmd('postgres flexible-server create -g {} -l {} --storage-size {} --public-access none'.format(
                  resource_group, location, invalid_storage_size),
@@ -269,28 +284,34 @@ def _test_mgmt_update_validator(self, resource_group):
                  .format(resource_group, server_name, location, tier, version, sku_name, storage_size, backup_retention))
         self.cmd('postgres flexible-server show -g {} -n {}'.format(resource_group, server_name), checks=list_checks)
 
+        # Update server with invalid tier for current sku
         invalid_tier = 'GeneralPurpose'
         self.cmd('postgres flexible-server update -g {} -n {} --tier {}'.format(
-                 resource_group, server_name, invalid_tier), # can't update to this tier because of the instance's sku name
+                 resource_group, server_name, invalid_tier),
                  expect_failure=True)
 
+        # Update server with invalid sku for the given tier
         self.cmd('postgres flexible-server update -g {} -n {} --tier {} --sku-name {}'.format(
                  resource_group, server_name, valid_tier, invalid_sku_name),
                  expect_failure=True)
 
+        # Update server with smaller than current storage size
         invalid_storage_size = 64
         self.cmd('postgres flexible-server update -g {} -n {} --storage-size {}'.format(
-                 resource_group, server_name, invalid_storage_size), #cannot update to smaller size
+                 resource_group, server_name, invalid_storage_size),
                  expect_failure=True)
 
+        # Update server with invalid backup retention days
         self.cmd('postgres flexible-server update -g {} -n {} --backup-retention {}'.format(
                  resource_group, server_name, invalid_backup_retention),
                  expect_failure=True)
 
+        # Update server with high availability on a tier that does not support it
         ha_value = 'ZoneRedundant'
         self.cmd('postgres flexible-server update -g {} -n {} --high-availability {}'.format(
                  resource_group, server_name, ha_value),
                  expect_failure=True)
 
+        # Delete server
         self.cmd('postgres flexible-server delete -g {} -n {} --yes'.format(
                  resource_group, server_name), checks=NoneCheck())

src/azure-cli/azure/cli/command_modules/postgresql/tests/latest/test_postgres_flexible_commands_advanced_threat_protection.py

@@ -25,7 +25,7 @@ def test_postgres_flexible_server_advanced_threat_protection_setting_mgmt(self,
         self._test_advanced_threat_protection_setting_mgmt(resource_group, server)
 
     def _test_advanced_threat_protection_setting_mgmt(self, resource_group, server):
-        # show advanced threat protection setting for server
+        # Confirm that advanced threat protection setting for server is disabled by default
         self.cmd('postgres flexible-server advanced-threat-protection-setting show -g {} --server-name {} '
                     .format(resource_group, server),
                     checks=[JMESPathCheck('state', "Disabled")]).get_output_in_json()
@@ -36,7 +36,8 @@ def _test_advanced_threat_protection_setting_mgmt(self, resource_group, server):
 
         os.environ.get(ENV_LIVE_TEST, False) and sleep(2 * 60)
 
-        # show advanced threat protection setting for server
+        # Confirm that advanced threat protection setting for server is enabled
+
         self.cmd('postgres flexible-server advanced-threat-protection-setting show -g {} --server-name {} '
                     .format(resource_group, server),
                     checks=[JMESPathCheck('state', "Enabled")]).get_output_in_json()
@@ -47,7 +48,7 @@ def _test_advanced_threat_protection_setting_mgmt(self, resource_group, server):
 
         os.environ.get(ENV_LIVE_TEST, False) and sleep(2 * 60)
 
-        # show advanced threat protection setting for server
+        # Confirm that advanced threat protection setting for server is disabled
         self.cmd('postgres flexible-server advanced-threat-protection-setting show -g {} --server-name {} '
                     .format(resource_group, server),
                     checks=[JMESPathCheck('state', "Disabled")]).get_output_in_json()

src/azure-cli/azure/cli/command_modules/postgresql/tests/latest/test_postgres_flexible_commands_backup.py

@@ -43,22 +43,24 @@ def _test_backups_mgmt(self, resource_group, server):
 
         self.assertDictEqual(automatic_backup, backups[0])
 
-        # test on-demand backup create
+        # Create on-demand backup
         backup_name = self.create_random_name(F'backup', 16)
 
         self.cmd('postgres flexible-server backup create -g {} -n {} --backup-name {}'
                 .format(resource_group, server, backup_name),
                 checks=[JMESPathCheck('name', backup_name)])
 
+        # Confirm that on-demand backup is created
         backups_update = self.cmd('postgres flexible-server backup list -g {} -n {}'
                         .format(resource_group, server)).get_output_in_json()
 
         self.assertTrue(backups_length < len(backups_update))
 
-        # test on-demand backup delete
+        # Delete on-demand backup
         self.cmd('postgres flexible-server backup delete -g {} -n {} --backup-name {} --yes'
                 .format(resource_group, server, backup_name))
 
+        # Confirm that on-demand backup is deleted
         backups_update = self.cmd('postgres flexible-server backup list -g {} -n {}'
                         .format(resource_group, server)).get_output_in_json()
 

src/azure-cli/azure/cli/command_modules/postgresql/tests/latest/test_postgres_flexible_commands_elastic_clusters.py

@@ -33,7 +33,7 @@ def _test_elastic_clusters_mgmt(self, resource_group):
         cluster_name = self.create_random_name(SERVER_NAME_PREFIX, SERVER_NAME_MAX_LENGTH)
         cluster_size = 2
 
-        # create elastic cluster
+        # Create elastic cluster
         self.cmd('postgres flexible-server create -g {} -n {} --sku-name {} \
                    --version {} --cluster-option ElasticCluster --public-access Enabled'
                   .format(resource_group, cluster_name, sku_name, version))
@@ -47,7 +47,7 @@ def _test_elastic_clusters_mgmt(self, resource_group):
         self.assertEqual(basic_info['version'], version)
         self.assertEqual(basic_info['cluster']['clusterSize'], cluster_size)
 
-        # test failures
+        # Test failures
         self.cmd('postgres flexible-server update -g {} -n {} --storage-auto-grow Enabled'
                  .format(resource_group, cluster_name), expect_failure=True)
         self.cmd('postgres flexible-server update -g {} -n {} --node-count {}'
@@ -57,7 +57,7 @@ def _test_elastic_clusters_mgmt(self, resource_group):
         self.cmd('postgres flexible-server db create -g {} -s {} -d dbclusterfail'
                  .format(resource_group, cluster_name), expect_failure=True)
 
-        # update cluster
+        # Update cluster
         update_cluster_size = 4
         update_info = self.cmd('postgres flexible-server update -g {} -n {} --node-count {}'
                                .format(resource_group, cluster_name, update_cluster_size)).get_output_in_json()
@@ -73,6 +73,6 @@ def _test_elastic_clusters_mgmt(self, resource_group):
         self.assertEqual(restore_result['name'], cluster_restore_name)
         self.assertEqual(restore_result['cluster']['clusterSize'], update_cluster_size)
 
-        # delete everything
+        # Clean up
         self.cmd('postgres flexible-server delete -g {} -n {} --yes'.format(resource_group, cluster_name))
         self.cmd('postgres flexible-server delete -g {} -n {} --yes'.format(resource_group, cluster_restore_name))

src/azure-cli/azure/cli/command_modules/postgresql/tests/latest/test_postgres_flexible_commands_fabric_mirroring.py

@@ -22,17 +22,17 @@ def test_postgres_flexible_server_fabric_mirroring_mgmt(self, resource_group, se
 
 
     def _test_fabric_mirroring_mgmt(self, resource_group, server):
-        # create a database
+        # Create a database
         database2_name = 'flexibleserverdb'
         self.cmd('postgres flexible-server db create -g {} -s {} -d {}'.format(resource_group, server, database2_name),
                  checks=[JMESPathCheck('name', database2_name)])
 
-        # enable system assigned managed identity
+        # Enable system assigned managed identity
         self.cmd('postgres flexible-server identity update -g {} -s {} --system-assigned Enabled'
                  .format(resource_group, server),
                  checks=[JMESPathCheck('type', 'SystemAssigned')])
 
-        # enable fabric mirroring
+        # Enable fabric mirroring
         database1 = 'postgres'
         self.cmd('postgres flexible-server fabric-mirroring start -g {} --server-name {} --database-names {} --yes'
                     .format(resource_group, server, database1))
@@ -41,12 +41,12 @@ def _test_fabric_mirroring_mgmt(self, resource_group, server):
         self.cmd('postgres flexible-server parameter show --name azure.mirror_databases -g {} -s {}'.format(resource_group, server),
                  checks=[JMESPathCheck('value', database1)])
 
-        # update mirrored database
+        # Update mirrored database
         self.cmd('postgres flexible-server fabric-mirroring update-databases -g {} --server-name {} --database-names {} --yes'
                  .format(resource_group, server, database2_name),
                  checks=[JMESPathCheck('value', database2_name)])
 
-        # disable fabric mirroring
+        # Disable fabric mirroring
         self.cmd('postgres flexible-server fabric-mirroring stop -g {} --server-name {} --yes'
                  .format(resource_group, server))
         self.cmd('postgres flexible-server parameter show --name azure.fabric_mirror_enabled -g {} -s {}'.format(resource_group, server),