improve messaging

Author: NedAnd1

src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_helpers.py

@@ -291,8 +291,10 @@ def get_extension_installed_and_cluster_configs(
 
 
 def should_delete_extension(storage_options_to_remove) -> bool:
-    return (storage_options_to_remove in [True, CONST_ACSTOR_ALL] or
-            isinstance(storage_options_to_remove, list) and CONST_ACSTOR_ALL in storage_options_to_remove)
+    return (
+        storage_options_to_remove in [True, CONST_ACSTOR_ALL] or
+        (isinstance(storage_options_to_remove, list) and CONST_ACSTOR_ALL in storage_options_to_remove)
+    )
 
 
 def get_container_storage_extension_installed(

src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py

@@ -464,13 +464,14 @@ def validate_enable_azure_container_storage_params(
         for option in enablement_option_arr:
             if option not in [CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN]:
                 raise InvalidArgumentValueError(
-                    f"Unsupported Azure Container Storage option '{option}'. "
+                    f"Unsupported storage option '{option}'. "
                     f"Supported values are '{CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK}' "
                     f"and '{CONST_STORAGE_POOL_TYPE_ELASTIC_SAN}'."
                 )
         if is_ephemeral_disk_enabled == enable_ephemeral_disk and is_elastic_san_enabled == enable_elastic_san:
+            options_display = "', '".join(enablement_option_arr)
             raise InvalidArgumentValueError(
-                f"Cannot enable Azure Container Storage option(s) '{enablement_option}' "
+                f"Cannot enable the requested storage options ('{options_display}') "
                 "as they are already enabled on the cluster."
             )
 
@@ -530,13 +531,14 @@ def validate_disable_azure_container_storage_params(
                 actionable = actionable or is_elastic_san_enabled
             else:
                 raise InvalidArgumentValueError(
-                    f"Cannot disable unsupported Azure Container Storage option '{disable_option}'. "
+                    f"Cannot disable unsupported storage option '{disable_option}'. "
                     f"Supported values are '{CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK}', "
                     f"'{CONST_STORAGE_POOL_TYPE_ELASTIC_SAN}' and '{CONST_ACSTOR_ALL}'."
                 )
         if not actionable:
+            options_display = "', '".join(disablement_option_arr)
             raise InvalidArgumentValueError(
-                f"Cannot disable Azure Container Storage option(s) '{disablement_option}' "
+                f"Cannot disable the requested storage options ('{options_display}') "
                 "as they could not be found on the cluster."
             )
 

src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py

@@ -9272,15 +9272,19 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
                     )
                     from azure.cli.command_modules.acs.azurecontainerstorage._helpers import should_delete_extension
                     if not should_delete_extension(disable_azure_container_storage_param):
-                        storage_option_str = disable_azure_container_storage_param
+                        storage_option_display = storage_option_param_str = disable_azure_container_storage_param
                         if isinstance(disable_azure_container_storage_param, list):
-                            storage_option_str = " ".join(disable_azure_container_storage_param)
+                            storage_options = disable_azure_container_storage_param
+                            storage_option_param_str = " ".join(storage_options)
+                            if len(storage_options) > 2:
+                                storage_options = [storage_options[:-1].join("', '"), storage_options[-1]]
+                            storage_option_display = "' and '".join(storage_options)
                         msg = (
                             "Please make sure there are no existing PVs and PVCs that are provisioned by Azure Container Storage "
-                            f"for {disable_azure_container_storage_param} before disabling. If storage options are disabled "
+                            f"for '{storage_option_display}' before disabling. If storage options are disabled "
                             "with remaining PVs and PVCs, any data associated with those PVs and PVCs will not be erased "
                             "and the nodes will be left in an unclean state. The PVs and PVCs can only be cleaned up "
-                            f"after re-enabling it by running 'az aks update --enable-azure-container-storage {storage_option_str}'. "
+                            f"after re-enabling it by running 'az aks update --enable-azure-container-storage {storage_option_param_str}'. "
                             "Would you like to proceed with the disabling?"
                         )
                     if not self.context.get_yes() and not prompt_y_n(msg, default="n"):

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_validators.py

@@ -1641,7 +1641,7 @@ def test_disable_unsupported_type(self):
                 storage_pool_type, True, False, False, "", "", "", "",
             )
         err = (
-            f"Cannot disable unsupported Azure Container Storage option '{storage_pool_type}'. "
+            f"Cannot disable unsupported storage option '{storage_pool_type}'. "
             "Supported values are 'ephemeralDisk', 'elasticSan' and 'all'."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1653,7 +1653,7 @@ def test_disable_already_disabled_type(self):
                 storage_pool_type, True, False, False, "", "", "", "",
             )
         err = (
-            f"Cannot disable Azure Container Storage option(s) '{storage_pool_type}' "
+            f"Cannot disable the requested storage options ('{storage_pool_type}') "
             "as they could not be found on the cluster."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1770,7 +1770,7 @@ def test_enable_supported_type_when_already_enabled(self):
                 storage_type, True, True, False, False, "", None, None, None, None,
             )
         err = (
-            f"Cannot enable Azure Container Storage option(s) '{storage_type}' "
+            f"Cannot enable the requested storage options ('{storage_type}') "
             "as they are already enabled on the cluster."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1794,7 +1794,7 @@ def test_enable_multiple_types_when_all_are_already_enabled(self):
                 storage_types, True, True, True, False, "", None, None, None, None,
             )
         err = (
-            f"Cannot enable Azure Container Storage option(s) '{storage_types}' "
+            f"Cannot enable the requested storage options ('{storage_types[0]}', '{storage_types[1]}') "
             "as they are already enabled on the cluster."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1808,7 +1808,7 @@ def test_enable_multiple_types_when_some_are_unsupported(self):
                 storage_types, False, False, False, False, "", None, None, None, None,
             )
         err = (
-            f"Unsupported Azure Container Storage option '{unsupported_storage_type}'. "
+            f"Unsupported storage option '{unsupported_storage_type}'. "
             "Supported values are 'ephemeralDisk' and 'elasticSan'."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1820,7 +1820,7 @@ def test_enable_unsupported_type(self):
                 storage_type, False, False, False, False, "", None, None, None, None,
             )
         err = (
-            f"Unsupported Azure Container Storage option '{storage_type}'. "
+            f"Unsupported storage option '{storage_type}'. "
             "Supported values are 'ephemeralDisk' and 'elasticSan'."
         )
         self.assertEqual(str(cm.exception), err)
@@ -1850,7 +1850,7 @@ def test_enable_with_storage_pool_sku(self):
             f'Please remove --storage-pool-sku {storage_pool_sku} from the command and try again.'
         )
         self.assertEqual(str(cm.exception), err)
-        
+
     def test_enable_with_storage_pool_option(self):
         storage_pool_option = "valid-option"
         with self.assertRaises(InvalidArgumentValueError) as cm: