Repository navigation
[AppService] az appservice plan create: Fix Windows defaults for App Service managed instance plans - #34110
Open
Andrew Westgarth (apwestgarth) wants to merge 3 commits into
Conversation
Default App Service plans to Windows when --is-managed-instance is specified without --is-linux. Reject unsupported argument combinations: - --is-managed-instance with --is-linux true - --hyper-v with --is-managed-instance Update parameter help and add unit tests covering the managed instance Windows default and mutual-exclusion validation.
Andrew Westgarth (apwestgarth)
requested a review
from a team
as a code owner
September 21, 2026 18:40
|
Hi Andrew Westgarth (@apwestgarth), |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The registered validator still overrides the managed-instance default and emits the existing conflict message; current tests bypass that command path.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Updates az appservice plan create to support Windows defaults for managed-instance plans and reject unsupported option combinations.
Changes:
- Adds managed-instance defaulting and validation.
- Updates parameter help text.
- Adds focused unit tests.
| File | Summary |
|---|---|
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py |
Adds tests for defaults and validation. |
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Adds managed-instance handling and conflict checks. |
src/azure-cli/azure/cli/command_modules/appservice/_params.py |
Updates parameter guidance. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Default App Service plans to Windows when --is-managed-instance is specified without --is-linux. Update both the command validator and create handler to reject unsupported argument combinations: - --is-managed-instance with --is-linux true - --hyper-v with --is-managed-instance - --hyper-v with --is-linux true Update parameter help and add focused validator and handler unit tests for the Windows defaults and mutual-exclusion rules.
Member
Author
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Collaborator
|
appservice |
Aditya Pujara (a0x1ab)
added
the
Request X Engineering Agent
x-engineering-agent
Bot
changed the title
az appservice plan create: ] - Fix Windows defaults for App Service managed instance plans
x-engineering-agent
Bot
changed the title
az appservice plan create: ] - Fix Windows defaults for App Service managed instance plansaz appservice plan create: Fix Windows defaults for App Service managed instance plans
![x-engineering-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
x-engineering-agent
Bot
added
the
X Engineering Agent Reviewed
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Aditya Pujara (a0x1ab)
added
the
Request X Engineering Agent Live Test
Contributor
Live test results —
|
Contributor
There was a problem hiding this comment.
Andrew Westgarth (@apwestgarth)
Review
The App Service managed-instance Windows default and incompatible-argument checks are consistent across validation and creation. Upstream CI is now 50/51 passed: 0 PR-related failures, 1 unrelated failure, 0 uncertain failures. The previously identified regression-test weakness remains on this unchanged head.
Upstream CI
azdev-style
- Result: failure
- Relevance: Not PR-related
- Evidence:
acs/custom.py:1070:11: W0718: Catching too general exception Exception(also lines 2442 and 2460);acs/azurecontainerstorage/_validators.py:361:19: R0916: Too many boolean expressions in if statement (6/5);sql/custom.py:1779:0: W0613: Unused argument 'kwargs';azure-cli-core/.../commands/arm.py:178:76: W0622: Redefining built-in 'type'. The task reportsPylint: FAILED; none of these paths is among this PR's five changed AppService files. - Suggested fix: Re-run the azdev-style workflow. If these diagnostics recur, escalate to the affected component/check owners; do not modify unrelated source in this AppService PR.
- Verify: Confirm the replacement azdev-style run passes on this head; no unrelated source change is requested.
Test validation
- Live test: Passed. Workflow run
- Regression coverage: For
appservice: 2 focused test file(s) changed. Scenario coverage is unverified; review the issue's required conditions, test setup, and assertions before approval. No linked issue was found in the PR timeline; inspect the source issue and its required scenario manually.
Test semantic strength
In test_managed_instance_defaults_to_windows, lines 1781-1788, except Exception: pass lets the success-path test pass when creation fails after constructing the model. The textual reserved=False check does not establish that the creation client was invoked successfully.
Configure the client mocks for normal completion, remove the blanket exception catch, and assert reserved=False directly in the constructor kwargs plus the creation-client call. Re-run the two changed appservice mock-test files and confirm an injected unexpected creation exception fails this test. These are deterministic unit-test scenarios; no recording was changed or supplied to establish service-level coverage. The seven repository review tools reported no additional deterministic policy findings.
Risk assessment
17/100 · Low · High confidence
The Low rating is driven by public CLI behavior.
- Change scope: 5 changed files, 117 changed lines (
+105/-12), including 3 production files. - Affected components:
appservice - Risk drivers: public CLI behavior (+18)
- Regression evidence: Changed regression tests are included, reducing risk.
- Confidence: High because changed-line patches were available for every production file.
- Required review: Owning-squad review is recommended for
appservicebefore merge.
Posted by x-engineering-agent (Reviewer)
Remove blanket exception suppression and use real SDK models. Mock AAZ execution and verify Windows and managed-instance arguments reach the creation command, then assert the handler returns its result. Validation: - Webapp mock-test module: 150 passed - azdev style appservice: pylint and flake8 passed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba9a26c8-d8d4-465b-877e-b5aac1abe29f
Contributor
X Engineering Agent ReportLive test — Passed (182/182)PASS Selectors: Changed test files runWorkflow run: https://github.com/Azure/issue-sentinel/actions/runs/38657514698 Last 80 lines of azdev outputPosted by x-engineering-agent (Tester) |
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Aditya Pujara (a0x1ab)
removed
the
Request X Engineering Agent
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation — ️✔️ All clear
Default App Service plans to Windows when --is-managed-instance is specified without --is-linux.
Reject unsupported argument combinations:
Update parameter help and add unit tests covering the managed instance Windows default and mutual-exclusion validation.
Related command
az appservice plan create
Description
Recent change in July changed default OS on App Service Plan creation from Windows to Linux. Original change handled situations where user specified --hyper-v to signify the plan was for Windows Containers and therefore must default to Windows. However this change did not handle Managed Instance on Azure App Service (--is-managed-instance) which is only available on Windows OS. Equally added change to ensure that neither --hyper-v or --is-managed-instance can be accepted when --is-linux but also added check to ensure the --hyper-v cannot be set on --is-managed-instance as Windows Containers are not supported in Managed Instance on Azure App Service
Testing Guide
App Service Plan for Windows Containers should default to Windows(--is-linux false)
az appservice plan create -n -l ---sku P1v4 --hyper-v
Managed Instance on Azure App Service Plan should default to Windows (--is-linux false)
az appservice plan create -n -l --sku P1v4 --is-managed-instance
Managed Instance on Azure App Service does not support Windows Containers so the following command should error
az appservice plan create -n -l --sku P1v4 --is-managed-instance --hyper-v
Error Message: --hyper-v and --is-managed-instance cannot be used together because Windows Containers are not supported in Managed Instance on Azure App Service.
Windows Containers cannot be hosted in a Linux App Service plan so the following command should error
az appservice plan create -n -l --sku P1v4 --hyper-v --is-linux true
Error Message: --hyper-v creates a Windows plan and cannot be combined with --is-linux true. Omit --is-linux or use "--is-linux false".
Managed Instance does not support Linux so the following command should error
az appservice plan create -n -l --sku P1v4 --is-managed-instance --is-linux true
Error Message: --is-managed-instance creates a Windows plan and cannot be combined with --is-linux true. Omit --is-linux or use "--is-linux false".
History Notes
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.