[AppService] az functionapp flex-migration: Create commands to migrate CV1 apps to Flex#31865
Conversation
kamperiadis
requested review from
jsntcy,
yanzhudd and
zhoxing-ms
as code owners
️✔️AzureCLI-FullTest
|
|
Hi @kamperiadis, |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| functionapp flex-migration | sub group functionapp flex-migration added |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds new CLI commands for migrating Linux Consumption function apps to the Flex Consumption plan. The feature provides automated migration capabilities to help customers transition from Linux Consumption (CV1) to Flex Consumption plans.
- Adds
az functionapp flex-migration listcommand to identify eligible function apps for migration - Adds
az functionapp flex-migration startcommand to perform the actual migration from CV1 to Flex - Includes comprehensive validation, configuration migration, and error handling capabilities
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/appservice/custom.py | Core migration logic with validation, app creation, and settings migration functions |
| src/azure-cli/azure/cli/command_modules/appservice/commands.py | Command registration for the new flex-migration command group |
| src/azure-cli/azure/cli/command_modules/appservice/_params.py | Parameter definitions and help text for the new migration commands |
| src/azure-cli/azure/cli/command_modules/appservice/_help.py | Documentation and examples for the migration commands |
| src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_functionapp_commands.py | Test cases for validating migration functionality |
kamperiadis
force-pushed
the
cv1toflexmigration
branch
6 times, most recently
from
f1dfff0 to
8797286
Compare
| subscription_id = get_subscription_id(cmd.cli_ctx) | ||
| web_client = get_mgmt_service_client(cmd.cli_ctx, WebSiteManagementClient) | ||
|
|
||
| logger.warning("Searching for function apps under the subscription '%s' that are eligible for Flex " |
There was a problem hiding this comment.
I know you told me that it appears ALL logs are being written using logger.warning, but that doesn't really make sense if you want to be able to have warnings stand out in output. So I guess we need general CLI guidance on why we shouldn't be using logger.info here, reserving warning/error for lines we want to stand out to the customer
There was a problem hiding this comment.
@zhoxing-ms Can you provide guidance on this?
According to the guidance document here "Log to logger.error() or logger.warning() for user messages". The next guideline is "Use the appropriate logging level for printing strings" but the example just highlights the distinction between using the logger vs returning the string as opposed to logger.info vs logger.warning.
When I switch to make these logs use logger.info instead of logger.warning, I do not see the logs unless I use the --verbose flag. It seems to be due to this dependency we use for the logging which will only show messages from warning logs and above, unless the --verbose and --debug flags are passed.
There was a problem hiding this comment.
If you want notification message to be displayed by default even without using parameter --verbose, you can try using print()
There was a problem hiding this comment.
Isn't that against what the guidelines outlined here: https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md#general-patterns
Log to logger.error() or logger.warning() for user messages; do not use the print() function
| _generic_site_operation(cmd.cli_ctx, resource_group, name, | ||
| 'update_scm_allowed', None, csmPublishingCredentialsPoliciesEntity) | ||
|
|
||
| logger.warning("Successfully enabled SCM basic publishing credentials") |
There was a problem hiding this comment.
you're using "enabled" rather than migrated on this and the next log, but others you're using "migrate"
There was a problem hiding this comment.
I figured "enabled" would make more sense in this context since it is just a single boolean property that we are enabling as opposed to a set of properties. What do you think?
| return update_configuration_polling(cmd, resource_group_name, name, slot, configs) | ||
| try: | ||
| return _generic_site_operation(cmd.cli_ctx, resource_group_name, name, 'update_configuration', slot, configs) | ||
| except Exception as ex: # pylint: disable=broad-exception-caught |
There was a problem hiding this comment.
Why the removal of this exception handler block?
There was a problem hiding this comment.
For context, this exception handling was added as part of this commit: ef54889
And this function is run with the az webapp config set command.
I do not think it is right that we are catching Conflict and Bad Request exceptions. I think that if either of those two exceptions are simply logged instead of thrown back to the client when run via a script in a pipeline, customers might miss that there were issues with updating the site config. Even for the purposes of this PR, the caller of this function should know via a thrown exception that something went wrong, and the caller should decide how to handle such exception.
| subnet_name=subnet_info["subnet_name"], | ||
| subnet_service_delegation=FLEX_SUBNET_DELEGATION if flexconsumption_location else None) | ||
| subnet_resource_id = subnet_info["subnet_resource_id"] | ||
| vnet_route_all_enabled = True |
There was a problem hiding this comment.
I don't have context on these changes - they don't seem related?
There was a problem hiding this comment.
The reason why I made these changes is because otherwise we get the "Readonly attribute name will be ignored in class" warning since vnet_route_all_enabled is only part of the SiteConfig class (code ref) It is not part of the Site class: https://github.com/Azure/azure-sdk-for-python/blob/afce6c3a2c8c1c47688e8d0c896da9eac07477b6/sdk/appservice/azure-mgmt-web/azure/mgmt/web/v2024_11_01/models/_models_py3.py#L19638
kamperiadis
force-pushed
the
cv1toflexmigration
branch
3 times, most recently
from
174337f to
083c619
Compare
kamperiadis
force-pushed
the
cv1toflexmigration
branch
from
083c619 to
fe3e709
Compare
kamperiadis
force-pushed
the
cv1toflexmigration
branch
from
fe3e709 to
6b12dc4
Compare
kamperiadis
force-pushed
the
cv1toflexmigration
branch
from
913cf42 to
61454ef
Compare
| ]) | ||
|
|
||
|
|
||
| class FunctionAppFlexMigrationTest(LiveScenarioTest): |
There was a problem hiding this comment.
May I ask why do you need to mark these tests as LiveScenarioTest?
There was a problem hiding this comment.
Much like what happened for this other PR #28001, because there are resources created on the customer's behalf (e.g. server farms, storage accounts, etc.), when I switch to using ScenarioTest, the resources that were generated in the yaml recording are different in the next run and it can't find a match. The tests run just fine when I set they run in live mode.
When using ScenarioTest, I get success the first time I run the tests. But then when I run those same tests again using the yaml recordings, I see things like this:
vcr.errors.CannotOverwriteExistingCassetteException: Can't overwrite existing cassette ('q:\src\azure-cli\src\azure-cli\azure\cli\command_modules\appservice\tests\latest\recordings\test_functionapp_flex_migration_site_configuration.yaml') in your current record mode ('once').
E No match for the request (<Request (PUT) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000002/providers/Microsoft.Web/serverfarms/ASP-clitestrg000002-0c0b?api-version=2024-11-01>) was found.
E Found 8 similar requests with 1 different matcher(s) :
E
E 1 - (<Request (PUT) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.Web/sites/srcfunc000005?api-version=2024-11-01>).
E Matchers succeeded : ['method', 'scheme', 'host', 'port', '_custom_request_query_matcher']
E Matchers failed :
E path - assertion failure :
E /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000002/providers/Microsoft.Web/serverfarms/ASP-clitestrg000002-0c0b != /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.Web/sites/srcfunc000005
E
E 2 - (<Request (PUT) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.Web/sites/srcfunc000005/config/appsettings?api-version=2024-11-01>).
E Matchers succeeded : ['method', 'scheme', 'host', 'port', '_custom_request_query_matcher']
E Matchers failed :
E path - assertion failure :
E /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000002/providers/Microsoft.Web/serverfarms/ASP-clitestrg000002-0c0b != /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.Web/sites/srcfunc000005/config/appsettings
Related command
az functionapp flex-migration listaz functionapp flex-migration startDescription
Linux Consumption customers have to run these steps manually in order to migrate their apps to Flex Consumption: https://learn.microsoft.com/en-us/azure/azure-functions/migration/migrate-plan-consumption-to-flex?tabs=azure-cli%2Clinux%2Csystem-assigned%2Ccontinuous%2Ctraces-table
With the
az functionapp flex-migration listcommand, customers will get a list of the Linux Consumption apps that are eligible for the migration to Flex.Additionally, with the
az functionapp flex-migration startcommand, customers will be able to create a Flex Consumption app from their Linux Consumption (CV1) app`. The command will migrate the relevant CV1 configurations to the Flex app. After running this migration command, customers will just need to deploy their code content, test it and then delete the original CV1 app.Testing Guide
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.