[Batch] BREAKING CHANGE: Remove deprecated properties (#32397)

Author: cRui861

src/azure-cli/azure/cli/command_modules/batch/_breaking_change.py

@@ -99,7 +99,7 @@
     'resource_file.blob_prefix',
     'resource_file.auto_storage_container_name',
     'virtual_machine_configuration.service_artifact_reference',
-
+    'target_node_communication_mode'
 ]
 
 # Options to be flattened into multiple arguments.

src/azure-cli/azure/cli/command_modules/batch/_parameter_format.py

@@ -17,7 +17,6 @@
 from azure.batch.models import (
     CachingType,
     BatchNodeDeallocationOption,
-    BatchNodeCommunicationMode,
     StorageAccountType)
 
 from azure.cli.core.commands.parameters import (
@@ -39,7 +38,6 @@
     keyvault_id,
     metadata_item_format,
     resource_file_format,
-    resource_tag_format,
     duration_format,
     storage_account_id,
     validate_client_parameters,
@@ -222,9 +220,6 @@ def load_arguments(self, _):
                                                                                             'configured on the Pool. If omitted, or if you specify an'
                                                                                             'empty collection, any existing metadata is removed from the'
                                                                                             'Pool.')
-        c.argument('target_node_communication_mode', options_list=['--target-communication'], arg_group='Pool',
-                   help="The desired node communication mode for the pool. If this element is present, it replaces the existing targetNodeCommunicationMode configured on the Pool. If omitted, any existing metadata is left unchanged.",
-                   arg_type=get_enum_type(BatchNodeCommunicationMode))
         c.argument('start_task_command_line', arg_group='Pool: Start Task',
                    help='The command line of the start task. The command line does not run under a shell, and therefore cannot take advantage of shell features such as environment variable expansion. If you want to take advantage of such features, you should invoke the shell in the command line, for example using "cmd /c MyCommand" in Windows or "/bin/sh -c MyCommand" in Linux.')
         c.argument('start_task_wait_for_success', action='store_true', arg_group='Pool: Start Task',
@@ -382,10 +377,6 @@ def load_arguments(self, _):
 
     with self.argument_context('batch pool create') as c:
         c.argument('json_file', help='The file containing pool create properties parameter specification in JSON(formatted to match REST API request body). If this parameter is specified, all \'Pool Create Properties Parameter Arguments\' are ignored.  See https://learn.microsoft.com/rest/api/batchservice/pool/add?tabs=HTTP#request-body')
-        c.argument('resource_tags', arg_group='Pool', type=resource_tag_format, help="User is able to specify resource tags for the pool. Any resource created for the pool will then also be tagged by the same resource tags")
-        c.argument('target_node_communication_mode', options_list=['--target-communication'],
-                   help="The desired node communication mode for the pool. If this element is present, it replaces the existing targetNodeCommunicationMode configured on the Pool. If omitted, any existing metadata is left unchanged.",
-                   arg_type=get_enum_type(BatchNodeCommunicationMode))
         c.argument('enable_accelerated_networking', arg_type=get_three_state_flag(), options_list=['--accelerated-networking'], arg_group="Pool: Network Configuration",
                    help='Whether this pool should enable accelerated networking. Accelerated networking enables single root I/O virtualization (SR-IOV) to a VM, which may lead to improved networking performance. For more details, see: https://learn.microsoft.com/azure/virtual-network/accelerated-networking-overview. Set true to enable.')
         c.argument('caching',
@@ -433,11 +424,6 @@ def load_arguments(self, _):
         c.argument('prioritize_unhealthy_instances', arg_type=get_three_state_flag())
         c.argument('rollback_failed_instances_on_policy_breach', arg_type=get_three_state_flag())
 
-    with self.argument_context('batch pool set') as c:
-        c.argument('target_node_communication_mode', options_list=['--target-communication'],
-                   help="The desired node communication mode for the pool. If this element is present, it replaces the existing targetNodeCommunicationMode configured on the Pool. If omitted, any existing metadata is left unchanged.",
-                   arg_type=get_enum_type(BatchNodeCommunicationMode))
-
     with self.argument_context('batch task create') as c:
         c.argument('json_file', type=file_type, help='The file containing the task(s) to create in JSON(formatted to match REST API request body). When submitting multiple tasks, accepts either an array of tasks or a TaskAddCollectionParamater. If this parameter is specified, all other parameters are ignored.', validator=validate_json_file, completer=FilesCompleter())
         c.argument('application_package_references', nargs='+', help='The space-separated list of IDs specifying the application packages to be installed. Space-separated application IDs with optional version in \'id[#version]\' format.', type=batch_application_package_reference_format)

src/azure-cli/azure/cli/command_modules/batch/_params.py

@@ -23,6 +23,8 @@ def transform_object(self, result):
         new_dict = {}
         for key, value in result.items():
             new_key = self.transform_mapping[key] if key in self.transform_mapping else key
+            if new_key is None:
+                continue
             if isinstance(value, Mapping):
                 new_dict[new_key] = self.transform_result(value)
             else:
@@ -65,6 +67,10 @@ def transform_object_list(self, result):
     'userCPUTime': 'userCpuTime',
     'writeIOGiB': 'writeIoGiB',
     'writeIOps': 'writeIops',
+    # Deprecated properties
+    'targetNodeCommunicationMode': None,
+    'currentNodeCommunicationMode': None,
+    'resourceTags': None
 }
 
 batch_transformer = Transformer(transform_map)

src/azure-cli/azure/cli/command_modules/batch/_transformers.py

@@ -81,23 +81,6 @@ def string_dictionary_format(values):
     return result
 
 
-def resource_tag_format(values):
-    """Space-separated values in 'key=value' format."""
-    if not values:
-        raise ValueError("No values in resource tags. "
-                         "Argument values should be in the format a=b c=d")
-    result = {}
-    try:
-        for value in values.split(' '):
-            k, v = value.split('=')
-            result[k] = v
-    except ValueError:
-        message = ("Incorrectly formatted resource tags. "
-                   "Argument values should be in the format a=b c=d")
-        raise ValueError(message)
-    return result
-
-
 def environment_setting_format(value):
     """Space-separated values in 'key=value' format."""
     try:

src/azure-cli/azure/cli/command_modules/batch/_validators.py

@@ -475,7 +475,7 @@ def resize_pool(client, pool_id, target_dedicated_nodes=None, target_low_priorit
 
 def replace_pool(client,
                  pool_id, json_file=None, application_package_references=None,
-                 metadata=None, target_node_communication_mode=None, start_task_command_line=None,
+                 metadata=None, start_task_command_line=None,
                  start_task_environment_settings=None, start_task_max_task_retry_count=None,
                  start_task_resource_files=None, start_task_wait_for_success=None):
     if json_file:
@@ -501,8 +501,7 @@ def replace_pool(client,
             application_package_references = []
         param = BatchPoolReplaceContent(
             application_package_references=application_package_references,
-            metadata=metadata,
-            target_node_communication_mode=target_node_communication_mode)
+            metadata=metadata)
 
         if start_task_command_line:
             param.start_task = BatchStartTask(command_line=start_task_command_line,

src/azure-cli/azure/cli/command_modules/batch/custom.py

@@ -54,19 +54,6 @@ def test_batch_metadata_item_format(self):
 
         with self.assertRaises(ValueError):
             _validators.metadata_item_format("name=value=other")
-    
-    def test_batch_resource_tag_format(self):
-        resource_tag = _validators.resource_tag_format("name=value")
-        self.assertEqual(resource_tag, {'name': 'value'})
-
-        with self.assertRaises(ValueError):
-            _validators.resource_tag_format("test")
-
-        with self.assertRaises(ValueError):
-            _validators.resource_tag_format("name=value=other")
-
-        with self.assertRaises(ValueError):
-            _validators.resource_tag_format("")
 
     def test_batch_environment_setting_format(self):
         env = _validators.environment_setting_format("name=value")
@@ -604,7 +591,7 @@ def test_batch_load_arguments(self):
         # pylint: disable=too-many-statements
         handler = azure.batch._client.BatchClient.create_pool
         args = list(self.command_pool._load_transformed_arguments(handler))
-        self.assertEqual(len(args), 48)
+        self.assertEqual(len(args), 47)
         self.assertFalse('yes' in [a for a, _ in args])
         self.assertTrue('json_file' in [a for a, _ in args])
         self.assertFalse('destination' in [a for a, _ in args])

src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_commands.py

@@ -117,6 +117,10 @@ def test_batch_pool_cmd(
                            self.check('metadata[0].name', 'a'),
                            self.check('metadata[1].value', 'd')])
 
+        # test that deprecated --target-communication for pool reset
+        with self.assertRaises(SystemExit):
+            self.batch_cmd('batch pool reset --pool-id {p_id} --target-communication classic')
+
         self.batch_cmd('batch pool delete --pool-id {p_id} --yes')
 
     @ResourceGroupPreparer()
@@ -540,11 +544,18 @@ def test_batch_pools_and_nodes(
         # test create pool using parameters
         self.batch_cmd('batch pool create --id {pool_i} --vm-size Standard_DS1_v2 '
                        '--image Canonical:UbuntuServer:18.04-LTS '
-                       '--node-agent-sku-id "batch.node.ubuntu 18.04" '
-                       '--target-communication classic')
-        self.batch_cmd('batch pool show --pool-id {pool_i}').assert_with_checks([
-            self.check('targetNodeCommunicationMode', 'classic')
-        ])
+                       '--node-agent-sku-id "batch.node.ubuntu 18.04"')
+        
+        # test that deprecated --target-communication argument causes argparse error
+        with self.assertRaises(SystemExit):
+            self.batch_cmd('batch pool create --id test-deprecated-arg --vm-size Standard_DS1_v2 '
+                           '--image Canonical:UbuntuServer:18.04-LTS '
+                           '--node-agent-sku-id "batch.node.ubuntu 18.04" '
+                           '--target-communication classic')
+        
+        # test that the deprecated targetNodeCommunicationMode property is not included in pool show output
+        pool_result = self.batch_cmd('batch pool show --pool-id {pool_i}').get_output_in_json()
+        self.assertNotIn('targetNodeCommunicationMode', pool_result, 'targetNodeCommunicationMode should not be present in pool output')
 
         # test create pool with missing parameters
         with self.assertRaises(SystemExit):
@@ -634,15 +645,13 @@ def test_batch_pools_and_nodes(
         updated = self.batch_cmd('batch pool show --pool-id {pool_j} --select "startTask"').get_output_in_json()
         self.assertNotEqual(current['startTask']['commandLine'], updated['startTask']['commandLine'])
 
-        # test patch pool with target-node-communication-mode
-        self.batch_cmd('batch pool set --pool-id {pool_j} --target-communication classic')
-        self.batch_cmd('batch pool show --pool-id {pool_j}').assert_with_checks([
-            self.check('targetNodeCommunicationMode', 'classic')
-        ])
-
         # test list node agent skus
         self.batch_cmd('batch pool supported-images list')
 
+        # test deprecated --target-communication for pool set
+        with self.assertRaises(SystemExit):
+            self.batch_cmd('batch pool set --pool-id {pool_i} --target-communication classic')
+
         # test app package reference
         self.batch_cmd('batch pool set --pool-id {pool_i} --application-package-references does-not-exist',
                        expect_failure=True)
@@ -658,6 +667,9 @@ def test_batch_pools_and_nodes(
         self.assertEqual(len(node_list), 1)
         self.kwargs.update({'node1': node_list[0]['id']})
 
+        # test that deprecated currentNodeCommunicationMode property is not present in pool output
+        self.assertNotIn('currentNodeCommunicationMode', node_list, 'currentNodeCommunicationMode should not be present in pool output')
+
         # node show
         self.batch_cmd('batch node show --pool-id {pool_i} --node-id {node1}').assert_with_checks([
             self.check('id', self.kwargs['node1']),

src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py

@@ -405,14 +405,7 @@ def test_batch_byos_account_cmd(self, resource_group):
 
         self.batch_cmd('batch pool create --id xplatCreatedPool --vm-size "standard_d2s_v3" '
                         '--image "canonical:0001-com-ubuntu-server-focal:20_04-lts" '
-                        '--node-agent-sku-id "batch.node.ubuntu 20.04" '
-                        '--resource-tags "dept=finance env=prod"')
-
-        # test for resource tags
-        self.batch_cmd('batch pool show --pool-id xplatCreatedPool').assert_with_checks([
-            self.check('resourceTags.dept', 'finance'),
-            self.check('resourceTags.env', 'prod'),
-        ])
+                        '--node-agent-sku-id "batch.node.ubuntu 20.04" ')
 
         # test batch account delete
         self.cmd('batch account delete -g {rg} -n {byos_n} --yes')