[Network] az network virtual-appliance migration: --migration-type is optional

[Graves (Jordang3177)]

🤖 PR Validation — ⚠️ Review suggested

Breaking Changes	Tests
⚠️ None	️✔️ 130/130

⚠️AzureCLI-BreakingChangeTest

⚠️network

rule	cmd_name	rule_message	suggest_message
⚠️ 1009 - ParaPropRemove	network virtual-appliance migration commit	cmd network virtual-appliance migration commit update parameter migration_type: removed property required=True
⚠️ 1009 - ParaPropRemove	network virtual-appliance migration execute	cmd network virtual-appliance migration execute update parameter migration_type: removed property required=True
⚠️ 1009 - ParaPropRemove	network virtual-appliance migration prepare	cmd network virtual-appliance migration prepare update parameter migration_type: removed property required=True

Related command

az network virtual-appliance migration prepare
az network virtual-appliance migration execute
az network virtual-appliance migration commit

Description

Updates the Network Virtual Appliance ILB migration flow to support a null request body.

--migration-type is now optional for the prepare, execute, and commit commands. When omitted, Azure CLI sends no request body and the service defaults the migration type to MigrateToNewILBArchitecture.

Explicitly specifying --migration-type remains supported for other workflows such as MigrateToNewOSVersion.

Related AAZ PR: Azure/aaz#1088
Original Azure CLI PR: #33766

Testing Guide

Run an ILB migration without specifying --migration-type:

# Phase 1: Prepare the ILB migration
az network virtual-appliance migration prepare \
    --resource-group MyResourceGroup \
    --name MyNva

# Phase 2: Execute the prepared migration
az network virtual-appliance migration execute \
    --resource-group MyResourceGroup \
    --name MyNva

# Phase 3: Commit the migration
az network virtual-appliance migration commit \
    --resource-group MyResourceGroup \
    --name MyNva

Verify the provisioning state between phases:

az network virtual-appliance show \
    --resource-group MyResourceGroup \
    --name MyNva \
    --query provisioningState

The existing explicit request-body flow remains supported:

az network virtual-appliance migration prepare \
    --resource-group MyResourceGroup \
    --name MyNva \
    --migration-type MigrateToNewOSVersion \
    --marketplace-version <version>

Automated tests verify that:

• --migration-type is optional for all three phases.
• Omitting migration options serializes the request body as None.
• Explicit OS migration options continue to serialize the expected request body.
• The live migration scenario exercises the null-body ILB workflow.

History Notes

[Network] az network virtual-appliance migration: Allow ILB migration commands to omit --migration-type

---

This checklist is used to make sure that common guidelines for a pull request are followed.

• The PR title and description has followed the guideline in Submitting Pull Requests.

• I adhere to the Command Guidelines.

• I adhere to the Error Handling Guidelines.

[Copilot]

🔵 Needs a closer look

Null-body serialization and migration-option validation issues remain, with related test coverage gaps.

Pull request overview

Updates NVA migration commands to support omitted --migration-type and null request bodies for ILB migrations while preserving explicit OS migration requests.

Changes:

• Makes migration type optional for prepare, execute, and commit.
• Updates unit and live tests for null-body workflows.
• Preserves explicit migration payload support.

File summaries

File	Review
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_unit_tests.py	Nit (1): Add coverage asserting marketPlaceVersion for explicit OS migration.
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py	Exercises the ILB migration workflow without --migration-type.
src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_prepare.py	Moderate (1): Required command arguments can cause { "properties": {} } instead of a null body. Moderate (1): Reject marketplace version without an OS migration type. Nit (1): Add combined OS migration and marketplace-version coverage.
src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_execute.py	Moderate (1): Required command arguments can cause { "properties": {} } instead of a null body.
src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_commit.py	Moderate (1): Required command arguments can cause { "properties": {} } instead of a null body.

Review details

Suppressed comments (6)

src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_commit.py:179

• With a real invocation, ctx.args also contains the required network_virtual_appliance_name and resource_group. Because set_prop("properties", ...) runs before this mapping, the root builder becomes non-empty even when migration_type is undefined, so serialize_content sends {"properties": {}} instead of None; the new unit test's empty context misses this. Gate creation of properties on the migration field (and test with the required -n/-g arguments) so the documented null-body flow actually sends no body.

                properties.set_prop("migrationType", AAZStrType, ".migration_type")

src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_execute.py:179

• With a real invocation, ctx.args also contains the required network_virtual_appliance_name and resource_group. Because set_prop("properties", ...) runs before this mapping, the root builder becomes non-empty even when migration_type is undefined, so serialize_content sends {"properties": {}} instead of None; the new unit test's empty context misses this. Gate creation of properties on the migration field (and test with the required -n/-g arguments) so the documented null-body flow actually sends no body.

                properties.set_prop("migrationType", AAZStrType, ".migration_type")

src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_prepare.py:185

• This relaxation also affects prepare when --marketplace-version is supplied, but the new test only exercises a type-only body. Please add a Prepare case with MigrateToNewOSVersion plus a marketplace version and assert both migrationType and marketPlaceVersion; otherwise a regression in that request mapping would go unnoticed.

                properties.set_prop("migrationType", AAZStrType, ".migration_type")

src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_prepare.py:185

• With a real invocation, ctx.args also contains the required network_virtual_appliance_name and resource_group. Because set_prop("properties", ...) runs before this mapping, the root builder becomes non-empty even when both migration fields are undefined, so serialize_content sends {"properties": {}} instead of None; the new unit test's empty context misses this. Gate creation of properties on the migration fields (and test with the required -n/-g arguments) so the documented null-body flow actually sends no body.

                properties.set_prop("migrationType", AAZStrType, ".migration_type")

src/azure-cli/azure/cli/command_modules/network/aaz/latest/network/virtual_appliance/migration/_prepare.py:68

• Making --migration-type optional also permits prepare --marketplace-version <version> by itself. This then builds {"properties": {"marketPlaceVersion": ...}} rather than the null body used for the ILB default, even though the help says this option only applies to MigrateToNewOSVersion; the service can therefore receive an ambiguous or invalid migration request. Please reject this combination, or require the OS migration type whenever --marketplace-version is set, before serialization.

            help="The type of migration workflow to prepare for the Network Virtual Appliance. If omitted, the migration defaults to MigrateToNewILBArchitecture.",

src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_unit_tests.py:54

• The explicit-body test only sets migration_type; it never supplies marketplace_version or checks marketPlaceVersion in Prepare's payload. That is the prepare-specific OS migration option advertised by this PR, so a regression in its serialization would pass these tests. Add a Prepare-only case with both values and assert both serialized properties.

                    'migration_type': 'MigrateToNewOSVersion',
                })
                operation = object.__new__(operation_type)
                operation.ctx = ctx
                self.assertEqual(operation.content, {

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Yong Zhang (yonzhan)]

Network

[Ethan Yang (necusjz)]

Graves (@Jordang3177) better to have a new recording.