fix: use one big vnet and attach AKS clusters to it to avoid creating bastion multiple times#8646
Open
awesomenix wants to merge 1 commit into
Open
fix: use one big vnet and attach AKS clusters to it to avoid creating bastion multiple times#8646awesomenix wants to merge 1 commit into
awesomenix wants to merge 1 commit into
Conversation
Contributor
awesomenix
commented
Jun 5, 2026
awesomenix
commented
- Make everyone's life simple
- README has all the details, but idea is to use shared VNET and stop doing unnecessary activities multiple times.
awesomenix
requested review from
AbelHu,
Devinwong,
SriHarsha001,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
sulixu,
surajssd,
timmy-wright and
zachary-bailey
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the E2E harness to use a single shared per-location VNet and Bastion (in abe2e-{location}) and attaches all AKS clusters to per-cluster subnets inside that VNet, reducing repeated Bastion provisioning and centralizing shared infrastructure creation.
Changes:
- Add shared-infrastructure provisioning (
shared_infra.go) to create/ensure a shared VNet, Bastion, Firewall, and managed identity, plus auto-allocation of per-cluster subnets. - Update cluster creation and networking helpers to rely on
VnetSubnetID(BYO VNet/subnet) rather than discovering VNets in the MC_ resource group. - Update SSH-over-Bastion flow and documentation to reflect shared Bastion/VNet usage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/vmss.go | Switch Bastion SSH command to use shared Bastion name + shared RG. |
| e2e/shared_infra.go | Introduce shared VNet/Bastion/Firewall/identity creation and per-cluster subnet allocation/cleanup. |
| e2e/README.md | Document the new shared-infra architecture and updated scenario authoring pointers. |
| e2e/node_config.go | Adjust tenant ID derivation to handle user-assigned identity clusters. |
| e2e/kube.go | Read cluster subnet ID from agent pool VnetSubnetID instead of listing VNets. |
| e2e/cluster.go | Create per-cluster subnet after name hashing; use shared Bastion; derive VNet from subnet ID. |
| e2e/cache.go | Ensure all cached cluster creators configure the shared VNet before preparing clusters. |
| e2e/aks_model.go | Avoid hardcoded subnet CIDRs; use parsed VNet/subnet info when configuring firewall/isolated settings. |
Comment on lines
475
to
479
| subnetAddressPrefix := vnet.addressPrefix | ||
|
|
||
| subnetParameters := armnetwork.Subnet{ | ||
| ID: to.Ptr(subnetId), | ||
| Properties: &armnetwork.SubnetPropertiesFormat{ |
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
2579b61 to
738f4ae
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
738f4ae to
69c05f8
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
69c05f8 to
f928610
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
f928610 to
be31cd4
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
be31cd4 to
e71ee5d
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
e71ee5d to
8315a13
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
8315a13 to
1bb4eab
Compare
Comment on lines
+126
to
+130
| poller, err := config.Azure.Subnet.BeginCreateOrUpdate(ctx, rg, SharedVNetName, PESubnetName, armnetwork.Subnet{ | ||
| Properties: &armnetwork.SubnetPropertiesFormat{ | ||
| AddressPrefix: to.Ptr(PESubnetCIDR), | ||
| }, | ||
| }, nil) |
Comment on lines
+153
to
+155
| for _, subnet := range page.Value { | ||
| name := *subnet.Name | ||
| if !strings.HasPrefix(name, "aks-subnet-") { |
Comment on lines
+434
to
+437
| for _, s := range page.Value { | ||
| if s.Properties != nil && s.Properties.AddressPrefix != nil { | ||
| usedCIDRs[*s.Properties.AddressPrefix] = true | ||
| } |
Comment on lines
+359
to
+361
| // ensureClusterIdentity creates a user-assigned managed identity for AKS clusters | ||
| // and grants it Network Contributor on the subscription so it can manage route tables | ||
| // in both the shared VNet and the MC_ resource groups. |
Comment on lines
+557
to
+558
| toolkit.Logf(ctx, "using shared bastion %s in %s", SharedBastionName, sharedRG) | ||
| return NewBastion(config.Azure.Credential, config.Config.SubscriptionID, sharedRG, *sharedBastion.Properties.DNSName), nil |
Comment on lines
+599
to
+605
| if zone.Name == nil || *zone.Name != privateZoneName { | ||
| continue | ||
| } | ||
| zoneRG := resourceGroupFromID(*zone.ID) | ||
| if strings.EqualFold(zoneRG, sharedRG) { | ||
| continue | ||
| } |
Comment on lines
+630
to
+631
| toolkit.Logf(ctx, "deleting conflicting DNS zone link %s in %s/%s (points to shared VNet)", *link.Name, zoneRG, zoneName) | ||
| poller, err := config.Azure.VirutalNetworkLinksClient.BeginDelete(ctx, zoneRG, zoneName, *link.Name, nil) |
Comment on lines
+1054
to
+1059
| if privateEndpoint.Properties == nil || len(privateEndpoint.Properties.NetworkInterfaces) == 0 { | ||
| return fmt.Errorf("private endpoint has no network interfaces") | ||
| } | ||
|
|
||
| aRecords := make([]*armprivatedns.ARecord, len(ipAddresses)) | ||
| for i, ip := range ipAddresses { | ||
| aRecords[i] = &armprivatedns.ARecord{IPv4Address: &ip} | ||
| } | ||
| ttl := int64(10) | ||
| aRecordSet := armprivatedns.RecordSet{ | ||
| Properties: &armprivatedns.RecordSetProperties{ | ||
| TTL: &ttl, | ||
| ARecords: aRecords, | ||
| }, | ||
| } | ||
| _, err := config.Azure.RecordSetClient.CreateOrUpdate(ctx, nodeResourceGroup, privateZoneName, armprivatedns.RecordTypeA, *dnsConfig.Fqdn, aRecordSet, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create record set: %w", err) | ||
| } | ||
| nicID := *privateEndpoint.Properties.NetworkInterfaces[0].ID | ||
| nicName := nicID[strings.LastIndex(nicID, "/")+1:] |
Comment on lines
+1065
to
+1067
| // Each NIC IP config has a private IP and an associated FQDN from the | ||
| // PE's privateLinkServiceConnections. Create one A record per FQDN. | ||
| for _, ipConfig := range nic.Properties.IPConfigurations { |
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
1bb4eab to
0648508
Compare
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
0648508 to
7f6b2a6
Compare
Comment on lines
+115
to
118
| networkProfile.ServiceCidr = to.Ptr("172.16.0.0/16") | ||
| networkProfile.ServiceCidrs = []*string{ | ||
| networkProfile.ServiceCidr, | ||
| to.Ptr("fd12:3456:789a:1::/108"), |
Comment on lines
+135
to
+138
| poller, err := config.Azure.Subnet.BeginCreateOrUpdate(ctx, rg, SharedVNetName, PESubnetName, armnetwork.Subnet{ | ||
| Properties: &armnetwork.SubnetPropertiesFormat{ | ||
| AddressPrefix: to.Ptr(PESubnetCIDR), | ||
| }, |
Comment on lines
+389
to
+392
| existing, err := config.Azure.UserAssignedIdentities.Get(ctx, rg, SharedClusterIdentity, nil) | ||
| if err == nil { | ||
| return *existing.ID, *existing.Properties.TenantID, nil | ||
| } |
Comment on lines
+557
to
+558
| toolkit.Logf(ctx, "using shared bastion %s in %s", SharedBastionName, sharedRG) | ||
| return NewBastion(config.Azure.Credential, config.Config.SubscriptionID, sharedRG, *sharedBastion.Properties.DNSName), nil |
Comment on lines
+721
to
+723
| // Check if the record already exists with the correct IPs | ||
| existing, err := config.Azure.RecordSetClient.Get(ctx, resourceGroup, zoneName, armprivatedns.RecordTypeA, recordName, nil) | ||
| if err == nil && existing.Properties != nil && existing.Properties.ARecords != nil { |
Comment on lines
+1067
to
+1077
| for _, ipConfig := range nic.Properties.IPConfigurations { | ||
| if ipConfig.Properties == nil || ipConfig.Properties.PrivateIPAddress == nil { | ||
| continue | ||
| } | ||
| ip := *ipConfig.Properties.PrivateIPAddress | ||
|
|
||
| func addDNSZoneGroup(ctx context.Context, privateZone *armprivatedns.PrivateZone, nodeResourceGroup, privateZoneName, endpointName string) error { | ||
| groupName := strings.Replace(privateZoneName, ".", "-", -1) // replace . with - | ||
| _, err := config.Azure.PrivateDNSZoneGroup.Get(ctx, nodeResourceGroup, endpointName, groupName, nil) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
| dnsZonegroup := armnetwork.PrivateDNSZoneGroup{ | ||
| Name: to.Ptr(fmt.Sprintf("%s/default", privateZoneName)), | ||
| Properties: &armnetwork.PrivateDNSZoneGroupPropertiesFormat{ | ||
| PrivateDNSZoneConfigs: []*armnetwork.PrivateDNSZoneConfig{{ | ||
| Name: to.Ptr(groupName), | ||
| Properties: &armnetwork.PrivateDNSZonePropertiesFormat{ | ||
| PrivateDNSZoneID: privateZone.ID, | ||
| // The PE's CustomDNSConfigs or PrivateLinkServiceConnections tell us the | ||
| // FQDN, but they may be empty. Use the IP config's | ||
| // PrivateLinkConnectionProperties for the FQDN list. | ||
| if ipConfig.Properties.PrivateLinkConnectionProperties == nil || len(ipConfig.Properties.PrivateLinkConnectionProperties.Fqdns) == 0 { | ||
| continue |
| - go.mod | ||
| - go.sum | ||
| - e2e/ | ||
| - e2e/scenario_win_test.go |
… bastion multiple times
awesomenix
force-pushed
the
nishp/fast/prefetchopt
branch
from
7f6b2a6 to
9b2d401
Compare
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.