Add WithSystemNodePool() to customize AKS system node pool VM size#16312
Merged
mitchdenny merged 4 commits intomainfrom Apr 20, 2026
Merged
Add WithSystemNodePool() to customize AKS system node pool VM size#16312mitchdenny merged 4 commits intomainfrom
mitchdenny merged 4 commits intomainfrom
Conversation
Fixes #16283 - The AKS system node pool VM size was hardcoded to Standard_D2s_v5 with no override. Users whose subscriptions don't allow that SKU in their target region had no workaround. Added AddSystemNodePool() extension method on IResourceBuilder<AzureKubernetesEnvironmentResource> that replaces the default system pool configuration. Parameters mirror AddNodePool() (vmSize, minCount, maxCount) but always target the system pool. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16312Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16312" |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new fluent extension method on AzureKubernetesEnvironmentResource to let callers override the AKS system node pool VM size (and autoscaling bounds), addressing deployments that fail when the default SKU isn’t available in a given subscription/region.
Changes:
- Introduces
AddSystemNodePool(vmSize, minCount, maxCount)to replace the default system pool configuration. - Adds unit tests validating replacement behavior, chaining with
AddNodePool, and Bicep output. - Adds a new Verify snapshot asserting the customized VM size is emitted in generated Bicep.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentExtensions.cs | Adds AddSystemNodePool API that removes existing system pool configs and inserts a customized system pool. |
| tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesEnvironmentExtensionsTests.cs | Adds coverage for default replacement, multiple calls, chaining, and Bicep generation. |
| tests/Aspire.Hosting.Azure.Kubernetes.Tests/Snapshots/AzureKubernetesEnvironmentExtensionsTests.AddSystemNodePool_BicepReflectsCustomVmSize.verified.bicep | New snapshot verifying Bicep reflects the custom system pool VM size. |
Comment on lines
+223
to
+224
| ArgumentOutOfRangeException.ThrowIfNegative(minCount); | ||
| ArgumentOutOfRangeException.ThrowIfNegative(maxCount); |
There was a problem hiding this comment.
AddSystemNodePool currently allows minCount (and therefore count) to be 0, because it only checks for negative values. AKS system node pools must have at least 1 node; generating Bicep with count: 0/minCount: 0 for mode: 'System' will fail deployment. Consider validating minCount >= 1 (and implicitly maxCount >= 1) and updating/adding a unit test to cover the invalid 0 case.
Suggested change
| ArgumentOutOfRangeException.ThrowIfNegative(minCount); | |
| ArgumentOutOfRangeException.ThrowIfNegative(maxCount); | |
| ArgumentOutOfRangeException.ThrowIfLessThan(minCount, 1); | |
| ArgumentOutOfRangeException.ThrowIfLessThan(maxCount, 1); |
| @@ -0,0 +1,52 @@ | |||
| @description('The location for the resource(s) to be deployed.') | |||
There was a problem hiding this comment.
This snapshot file appears to include a UTF-8 BOM / leading U+FEFF character before @description (visible as an extra character at the start of line 1). Other .verified.bicep snapshots in this test project don’t include it; please remove the BOM so the snapshot content/encoding is consistent and avoids diff/test noise.
Suggested change
| @description('The location for the resource(s) to be deployed.') | |
| @description('The location for the resource(s) to be deployed.') |
Member
Author
|
See also #16313 which is complementary — it adds VM size discovery to help users find available SKUs in their subscription/region. |
…shot System node pools require at least 1 node. Changed validation from ThrowIfNegative to ThrowIfLessThan(1) for both minCount and maxCount. Added test for the zero minCount rejection. Removed UTF-8 BOM from the snapshot file for consistency with other snapshots. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
reviewed
Apr 20, 2026
The With* prefix better reflects that this method configures/replaces the existing system pool rather than adding a new one alongside it. Consistent with other configuration methods like WithSubnet, WithWorkloadIdentity, and WithContainerRegistry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/deployment-test |
Contributor
|
🚀 Deployment tests starting on PR #16312... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
davidfowl
approved these changes
Apr 20, 2026
Contributor
|
❌ Deployment E2E Tests failed — 25 passed, 6 failed, 0 cancelled View test results and recordings
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
changed the title
Contributor
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24647795995 |
This was referenced Apr 23, 2026
Closed
Closed
Closed
Closed
Closed
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.
Description
Adds
AddSystemNodePool()extension method to allow customizing the AKS system node pool VM size and autoscaling configuration.Problem: The system node pool VM size was hardcoded to
Standard_D2s_v5with no way to override it. Users whose subscriptions don't allow that SKU in their target region had no workaround besides deploying to a different region.Solution:
AddSystemNodePool(vmSize, minCount, maxCount)replaces the default system pool configuration. It mirrors the existingAddNodePool()API but always targets the system pool (named "system", modeSystem). Returns the environment builder for chaining since users don't schedule workloads on system pools.Fixes #16283
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: