diff --git a/src/migrate/HISTORY.rst b/src/migrate/HISTORY.rst index 0f1ce6cb3df..e159abb3024 100644 --- a/src/migrate/HISTORY.rst +++ b/src/migrate/HISTORY.rst @@ -3,6 +3,10 @@ Release History =============== +3.0.0b3 ++++++++++++++++ +* Fix edge case bugs with az migrate get-discovered-server. + 3.0.0b2 +++++++++++++++ * Added replication list, get and start migration commands. diff --git a/src/migrate/azext_migrate/__init__.py b/src/migrate/azext_migrate/__init__.py index d3c97683f01..65abfbf53f6 100644 --- a/src/migrate/azext_migrate/__init__.py +++ b/src/migrate/azext_migrate/__init__.py @@ -4,6 +4,7 @@ # -------------------------------------------------------------------------------------------- from azure.cli.core import AzCommandsLoader +from azext_migrate._help import helps # pylint: disable=unused-import class MigrateCommandsLoader(AzCommandsLoader): diff --git a/src/migrate/azext_migrate/_help.py b/src/migrate/azext_migrate/_help.py index c7dc430bf27..d0b02d6d962 100644 --- a/src/migrate/azext_migrate/_help.py +++ b/src/migrate/azext_migrate/_help.py @@ -26,7 +26,7 @@ environments. """ -helps['migrate local get-discovered-server'] = """ +helps['migrate get-discovered-server'] = """ type: command short-summary: Retrieve discovered servers from an Azure Migrate project. long-summary: | @@ -69,31 +69,31 @@ examples: - name: List all discovered servers in a project text: | - az migrate local get-discovered-server \\ + az migrate get-discovered-server \\ --project-name myMigrateProject \\ --resource-group myRG - name: Get a specific discovered server by name text: | - az migrate local get-discovered-server \\ + az migrate get-discovered-server \\ --project-name myMigrateProject \\ --resource-group myRG \\ --name machine-12345 - name: Filter discovered servers by display name text: | - az migrate local get-discovered-server \\ + az migrate get-discovered-server \\ --project-name myMigrateProject \\ --resource-group myRG \\ --display-name "web-server" - name: List VMware servers discovered by a specific appliance text: | - az migrate local get-discovered-server \\ + az migrate get-discovered-server \\ --project-name myMigrateProject \\ --resource-group myRG \\ --appliance-name myVMwareAppliance \\ --source-machine-type VMware - name: Get a specific server from a specific appliance text: | - az migrate local get-discovered-server \\ + az migrate get-discovered-server \\ --project-name myMigrateProject \\ --resource-group myRG \\ --appliance-name myAppliance \\ @@ -226,7 +226,7 @@ Space-separated list of NIC IDs to replicate from the source server. Use this for power user mode. - - name: --vm-name + - name: --target-vm-name short-summary: Name of the VM to be created. long-summary: > The name for the virtual machine @@ -377,13 +377,13 @@ Note: This command uses a preview API version and may experience breaking changes in future releases. parameters: - - name: --protected-item-id --id + - name: --protected-item-id short-summary: Full ARM resource ID of the protected item. long-summary: > The complete ARM resource ID of the protected item. If provided, --resource-group and --project-name are not required. This ID can be obtained from the 'list' or 'new' commands. - - name: --protected-item-name --name + - name: --protected-item-name short-summary: Name of the protected item (replicating server). long-summary: > The name of the protected item to retrieve. @@ -439,14 +439,14 @@ Note: This command uses a preview API version and may experience breaking changes in future releases. parameters: - - name: --target-object-id --id + - name: --target-object-id short-summary: Replicating server ARM ID to disable replication. long-summary: > Specifies the ARM resource ID of the replicating server for which replication needs to be disabled. The ID should be retrieved using a get or list command for replication items. - - name: --force-remove --force + - name: --force-remove short-summary: Force remove the replication. long-summary: > Specifies whether the replication needs to be @@ -486,7 +486,7 @@ Note: This command uses a preview API version and may experience breaking changes in future releases. parameters: - - name: --job-id --id + - name: --job-id short-summary: Job ARM ID for which details need to be retrieved. long-summary: > Specifies the full ARM resource ID of the job. @@ -502,7 +502,7 @@ long-summary: > The name of the Azure Migrate project. Required when using --resource-group. - - name: --job-name --name + - name: --job-name short-summary: Job identifier/name. long-summary: > The name of the specific job to retrieve. @@ -558,7 +558,7 @@ Note: This command uses a preview API version and may experience breaking changes in future releases. parameters: - - name: --protected-item-id --id + - name: --protected-item-id short-summary: Full ARM resource ID of the protected item to migrate. long-summary: > The complete ARM resource ID of the replicating server. diff --git a/src/migrate/azext_migrate/_params.py b/src/migrate/azext_migrate/_params.py index c86d470e025..8f9dfbd3283 100644 --- a/src/migrate/azext_migrate/_params.py +++ b/src/migrate/azext_migrate/_params.py @@ -193,7 +193,7 @@ def load_arguments(self, _): required=True) c.argument( 'project_name', - project_name_type, + options_list=['--project-name'], help='The name of the migrate project.', required=True) c.argument('subscription_id', subscription_id_type) diff --git a/src/migrate/azext_migrate/custom.py b/src/migrate/azext_migrate/custom.py index 9a855368466..b84908e694b 100644 --- a/src/migrate/azext_migrate/custom.py +++ b/src/migrate/azext_migrate/custom.py @@ -12,6 +12,7 @@ logger = get_logger(__name__) +# pylint: disable=too-many-locals def get_discovered_server(cmd, project_name, resource_group, @@ -23,6 +24,7 @@ def get_discovered_server(cmd, from azext_migrate.helpers._utils import APIVersion from azext_migrate.helpers._server import ( validate_get_discovered_server_params, + extract_machine_name_from_id, build_base_uri, fetch_all_servers, filter_servers_by_display_name, @@ -33,6 +35,10 @@ def get_discovered_server(cmd, validate_get_discovered_server_params( project_name, resource_group, source_machine_type) + # Extract machine name if a full resource ID was provided for --name + if name: + name = extract_machine_name_from_id(name) + # Use current subscription if not provided if not subscription_id: from azure.cli.core.commands.client_factory import \ @@ -44,34 +50,27 @@ def get_discovered_server(cmd, subscription_id, resource_group, project_name, appliance_name, name, source_machine_type) - # Use the correct API version + # Construct the full URI with appropriate API version + # Note: Azure Migrate API does not support OData $filter for machines endpoint + # We'll apply client-side filtering after fetching all results api_version = (APIVersion.Microsoft_OffAzure.value if appliance_name else APIVersion.Microsoft_Migrate.value) - - # Prepare query parameters - query_params = [f"api-version={api_version}"] - if not appliance_name and display_name: - query_params.append(f"$filter=displayName eq '{display_name}'") - - # Construct the full URI request_uri = ( f"{cmd.cli_ctx.cloud.endpoints.resource_manager}{base_uri}?" - f"{'&'.join(query_params)}" + f"api-version={api_version}" ) try: # Fetch all servers values = fetch_all_servers(cmd, request_uri, send_get_request) - # Apply client-side filtering for display_name when using site - # endpoints - if appliance_name and display_name: + # Apply client-side filtering for display_name + if display_name: values = filter_servers_by_display_name(values, display_name) # Format and display the discovered servers information for index, server in enumerate(values, 1): - server_info = extract_server_info(server, index) - print_server_info(server_info) + print_server_info(extract_server_info(server, index)) except Exception as e: logger.error("Error retrieving discovered servers: %s", str(e)) diff --git a/src/migrate/azext_migrate/helpers/_server.py b/src/migrate/azext_migrate/helpers/_server.py index 88e6b4d4257..5bf7a8b28e4 100644 --- a/src/migrate/azext_migrate/helpers/_server.py +++ b/src/migrate/azext_migrate/helpers/_server.py @@ -19,6 +19,30 @@ def validate_get_discovered_server_params(project_name, raise CLIError("source_machine_type is not 'VMware' or 'HyperV'.") +def extract_machine_name_from_id(name_or_id): + """ + Extract the machine name from a full resource ID or return the name as-is. + + Handles formats like: + - Full ID: /subscriptions/.../Machines/5939691e-5505-4016-b4cd-4fa2d862a975 + - Simple name: 5939691e-5505-4016-b4cd-4fa2d862a975 + """ + if not name_or_id: + return None + + # If it looks like a full resource ID, extract the machine name + if name_or_id.startswith('/subscriptions/') or name_or_id.startswith('/Subscriptions/'): + parts = name_or_id.rstrip('/').split('/') + # The machine name should be the last part after "machines" or "Machines" + for i, part in enumerate(parts): + if part.lower() == 'machines' and i + 1 < len(parts): + return parts[i + 1] + # If we can't find machines, return the last segment + return parts[-1] if parts else name_or_id + + return name_or_id + + def build_base_uri(subscription_id, resource_group_name, project_name, appliance_name, name, source_machine_type): """Build the base URI for the API request.""" @@ -66,26 +90,43 @@ def fetch_all_servers(cmd, request_uri, send_get_request): """Fetch all servers including paginated results.""" response = send_get_request(cmd, request_uri) data = response.json() - values = data.get('value', []) - while data.get('nextLink'): - response = send_get_request(cmd, data.get('nextLink')) - data = response.json() - values += data.get('value', []) - - return values + # Handle single item response (when fetching by name/ID) + # Single items have 'id' and 'properties' at root level, not 'value' + if 'value' in data: + values = data.get('value', []) + while data.get('nextLink'): + response = send_get_request(cmd, data.get('nextLink')) + data = response.json() + values += data.get('value', []) + return values + if 'id' in data and 'properties' in data: + # Single machine response - wrap in list + return [data] + return [] def filter_servers_by_display_name(servers, display_name): - """Filter servers by display name.""" + """Filter servers by display name or machine name.""" filtered = [] for server in servers: properties = server.get('properties', {}) + + # Check properties.displayName first if properties.get('displayName', '') == display_name: filtered.append(server) + continue + + # Also check discoveryData[0].machineName + discovery_data = properties.get('discoveryData', []) + if discovery_data: + machine_name = discovery_data[0].get('machineName', '') + if machine_name == display_name: + filtered.append(server) return filtered +# pylint: disable=too-many-locals def extract_server_info(server, index): """Extract server information from discovery data.""" properties = server.get('properties', {}) @@ -93,16 +134,16 @@ def extract_server_info(server, index): # Default values machine_name = "N/A" - machine_id = "N/A" + machine_id = server.get('id', 'N/A') ip_addresses_str = 'N/A' os_name = "N/A" boot_type = "N/A" os_disk_id = "N/A" if discovery_data: + # Format from Microsoft.Migrate/migrateprojects/machines latest_discovery = discovery_data[0] machine_name = latest_discovery.get('machineName', 'N/A') - machine_id = server.get('id', 'N/A') ip_addresses = latest_discovery.get('ipAddresses', []) ip_addresses_str = ', '.join(ip_addresses) if ip_addresses else 'N/A' os_name = latest_discovery.get('osName', 'N/A') @@ -114,6 +155,35 @@ def extract_server_info(server, index): disk_details = json.loads(disk_details_json) if disk_details: os_disk_id = disk_details[0].get("InstanceId", "N/A") + else: + # Format from Microsoft.OffAzure/VMwareSites/machines or HyperVSites/machines + machine_name = properties.get('displayName', 'N/A') + + # Try to get IP addresses from different locations + network_adapters = properties.get('networkAdapters', []) + if network_adapters: + all_ips = [] + for adapter in network_adapters: + ips = adapter.get('ipAddressList', []) + all_ips.extend(ips) + ip_addresses_str = ', '.join(all_ips) if all_ips else 'N/A' + + # Get OS info from guestOSDetails or operatingSystemDetails + guest_os = properties.get('guestOSDetails', {}) + if guest_os: + os_name = guest_os.get('osName', 'N/A') + else: + os_details = properties.get('operatingSystemDetails', {}) + os_name = os_details.get('osName', os_details.get('osType', 'N/A')) + + # Get firmware/boot type + firmware = properties.get('firmware', 'N/A') + boot_type = firmware.lower() if firmware and firmware != 'N/A' else 'N/A' + + # Get disk info + disks = properties.get('disks', []) + if disks: + os_disk_id = disks[0].get('uuid', disks[0].get('diskId', 'N/A')) return { 'index': index, diff --git a/src/migrate/azext_migrate/helpers/replication/list/_execute_list.py b/src/migrate/azext_migrate/helpers/replication/list/_execute_list.py index 16dca653c9a..e0254df06e7 100644 --- a/src/migrate/azext_migrate/helpers/replication/list/_execute_list.py +++ b/src/migrate/azext_migrate/helpers/replication/list/_execute_list.py @@ -211,11 +211,13 @@ def _format_protected_item(item): # Add custom properties if available if custom_properties: formatted_item['instanceType'] = custom_properties.get('instanceType', 'N/A') - formatted_item['sourceMachineName'] = custom_properties.get('sourceMachineName', 'N/A') formatted_item['targetVmName'] = custom_properties.get('targetVmName', 'N/A') formatted_item['targetResourceGroupId'] = custom_properties.get('targetResourceGroupId', 'N/A') formatted_item['customLocationRegion'] = custom_properties.get('customLocationRegion', 'N/A') + # Use sourceVmName from API response (the actual VM display name) + formatted_item['sourceMachineName'] = custom_properties.get('sourceVmName', 'N/A') + return formatted_item diff --git a/src/migrate/azext_migrate/tests/latest/test_migrate_commands.py b/src/migrate/azext_migrate/tests/latest/test_migrate_commands.py index e6d615e04d6..be9127fc22c 100644 --- a/src/migrate/azext_migrate/tests/latest/test_migrate_commands.py +++ b/src/migrate/azext_migrate/tests/latest/test_migrate_commands.py @@ -101,10 +101,12 @@ def test_get_discovered_server_list_all(self, mock_get_sub_id, @mock.patch( 'azext_migrate.helpers._server.fetch_all_servers') + @mock.patch( + 'azext_migrate.helpers._server.filter_servers_by_display_name') @mock.patch( 'azure.cli.core.commands.client_factory.get_subscription_id') def test_get_discovered_server_with_display_name_filter( - self, mock_get_sub_id, mock_fetch_servers): + self, mock_get_sub_id, mock_filter_servers, mock_fetch_servers): """Test filtering discovered servers by display name""" from azext_migrate.custom import ( get_discovered_server) @@ -112,8 +114,10 @@ def test_get_discovered_server_with_display_name_filter( mock_get_sub_id.return_value = self.mock_subscription_id target_display_name = "WebServer" # Mock fetch_all_servers to return server data directly - mock_fetch_servers.return_value = [self._create_sample_server_data( - 1, "machine-1", target_display_name)] + sample_server = self._create_sample_server_data( + 1, "machine-1", target_display_name) + mock_fetch_servers.return_value = [sample_server] + mock_filter_servers.return_value = [sample_server] mock_cmd = self._create_mock_cmd() @@ -124,10 +128,10 @@ def test_get_discovered_server_with_display_name_filter( display_name=target_display_name ) - # Verify the filter was applied in the URL - call_args = mock_fetch_servers.call_args - self.assertIn("$filter", call_args[0][1]) - self.assertIn(target_display_name, call_args[0][1]) + # Verify client-side filtering was applied (API doesn't support $filter) + mock_filter_servers.assert_called_once() + filter_call_args = mock_filter_servers.call_args + self.assertEqual(filter_call_args[0][1], target_display_name) @mock.patch( 'azext_migrate.helpers._server.fetch_all_servers') diff --git a/src/migrate/linter_exclusions.yml b/src/migrate/linter_exclusions.yml index 5d1f7b924c9..973db3bb631 100644 --- a/src/migrate/linter_exclusions.yml +++ b/src/migrate/linter_exclusions.yml @@ -13,11 +13,13 @@ migrate local replication: rule_exclusions: - missing_group_help -migrate local get-discovered-server: +migrate get-discovered-server: rule_exclusions: - missing_command_test_coverage - missing_parameter_test_coverage - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule parameters: resource_group_name: rule_exclusions: @@ -28,6 +30,8 @@ migrate local replication init: - missing_command_test_coverage - missing_parameter_test_coverage - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule parameters: resource_group_name: rule_exclusions: @@ -38,19 +42,49 @@ migrate local replication new: - missing_command_test_coverage - missing_parameter_test_coverage - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule parameters: resource_group_name: rule_exclusions: - parameter_should_not_end_in_resource_group +migrate local replication list: + rule_exclusions: + - missing_command_test_coverage + - missing_parameter_test_coverage + - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule + +migrate local replication get: + rule_exclusions: + - missing_command_test_coverage + - missing_parameter_test_coverage + - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule + migrate local replication get-job: rule_exclusions: - missing_command_test_coverage - missing_parameter_test_coverage - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule migrate local replication remove: rule_exclusions: - missing_command_test_coverage - missing_parameter_test_coverage - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule + +migrate local start-migration: + rule_exclusions: + - missing_command_test_coverage + - missing_parameter_test_coverage + - missing_command_example + - missing_command_help + - unrecognized_help_parameter_rule diff --git a/src/migrate/setup.py b/src/migrate/setup.py index 9f7f4c19942..961f84010e9 100644 --- a/src/migrate/setup.py +++ b/src/migrate/setup.py @@ -7,7 +7,7 @@ from setuptools import setup, find_packages -VERSION = "3.0.0b2" +VERSION = "3.0.0b3" CLASSIFIERS = [ 'Development Status :: 4 - Beta',