azure: add multi-NIC support for external pod networking - #3233
azure: add multi-NIC support for external pod networking#3233manjotsidhu wants to merge 5 commits into
Conversation
Attach a second NIC to the Azure Pod VM so pod traffic to destinations outside the cluster can go directly over the Azure VNet instead of through the worker node's VXLAN tunnel. This builds on the existing EXTERNAL_NETWORK_VIA_PODVM mechanism already used by the AWS and Alibaba Cloud providers. - Declare both NICs in the VM creation request (AZURE_SECONDARY_SUBNET_ID), since Azure doesn't support hot-attaching a NIC without first deallocating the VM, unlike AWS's post-launch ENI attach. - Order returned IPs so the primary (control-plane) NIC's IPs always come first, regardless of the order Azure returns NICs in. - Fail CreateInstance fast with a clear error if multi-NIC pod networking is requested but AZURE_SECONDARY_SUBNET_ID isn't set, instead of deferring to a runtime failure deep in pod network setup. - Extend podnetwork's secondary-interface discovery (shared with AWS/Alibaba) to infer a gateway when the secondary NIC has no DHCP-assigned default route and isn't on the primary's subnet, which is the case on Azure since the two NICs sit on different subnets by design. Co-authored-by: devansh1109 <devanshverma911@gmail.com> Signed-off-by: Manjot-Sidhu <manjotsidhu@ibm.com>
|
@stevenhorsman or anyone from maintainers can help approve workflow checks ? |
Pradipta approved the check, but |
The test's secondary interface (eth1) was created as a Linux bridge, which cannot be moved between network namespaces (SetNamespace fails with EINVAL) — that's why every other movable interface in this test suite is a veth endpoint instead. Switch eth1 to a veth pair anchored by a throwaway peer namespace, matching that convention. This only affects the test harness; getSecondaryInterfaceDetails and inferGatewayFromSubnet themselves passed in CI. Signed-off-by: Manjot-Sidhu <manjotsidhu@ibm.com>
|
This PR is ready for review. |
| POD_SUBNET_CIDRS: "10.128.0.0/14,172.30.0.0/16,100.64.0.0/16" | ||
| ``` | ||
|
|
||
| - `AZURE_SECONDARY_SUBNET_ID` is required to enable multi-NIC. It must be a |
There was a problem hiding this comment.
is it maybe better to call this AZURE_EXTERNAL_SUBNET_ID to indicate what it's used for?
mkulke
left a comment
mkulke
left a comment
There was a problem hiding this comment.
I don't love this feature, this breaks certain kubernetes contracts/expecations for pods and has the potential to be a security footgun, for example network policies will not be respected, e.g. if I restrict egress traffic for my pod, this will undermine it. A more sophisticated version of this could maybe involve a controller that attempts to reconcile k8s network policies with security groups on the secondary nic.
In any case, the feature exists right now for other providers, and there's no good reason to not have it for azure.
we probably want to amend the comment here
(and maybe a remark about the security implications)
…document security implications Address review feedback from @mkulke on PR confidential-containers#3233: - Rename AZURE_SECONDARY_SUBNET_ID to AZURE_EXTERNAL_SUBNET_ID (and the Config field/flag to match) so the name reflects what the subnet is used for, consistent with the existing EXTERNAL_NETWORK_VIA_PODVM feature naming, rather than the "secondary NIC" implementation detail. - Add a security-considerations note to the shared docs/external-network.md: traffic routed via the secondary NIC bypasses the CNI plugin on the worker node, so NetworkPolicy egress rules do not apply to it. This applies to the existing AWS/Alibaba implementations too, not just Azure. - Cross-link that note from docs/azure-multi-nic.md. Signed-off-by: Manjot-Sidhu <manjotsidhu@ibm.com>
|
Updated the PR with the changes as per your review. |
| @@ -0,0 +1,96 @@ | |||
| # Azure multi-NIC pod networking | |||
There was a problem hiding this comment.
nit: I think this doc should go under src/cloud-api-adaptor/docs
| // NIC (on ExternalSubnetID) carries the Pod VM's external traffic and | ||
| // optionally gets a public IP if UsePublicIP is set. | ||
| func (p *azureProvider) buildNetworkConfigs(instanceName string, multiNic bool) []*armcompute.VirtualMachineNetworkInterfaceConfiguration { | ||
| primary := p.buildNetworkConfig(fmt.Sprintf("%s-net", instanceName), p.serviceConfig.SubnetID, true, p.serviceConfig.UsePublicIP && !multiNic) |
There was a problem hiding this comment.
What happens when EXTERNAL_NETWORK_VIA_PODVM=true and AZURE_USE_PUBLIC_IP=true? I think the worker node ends up dialing the secondary (external) NIC instead of the primary.
5 participants
Summary
Attaches a second NIC to the Azure Pod VM so that pod traffic to destinations
outside the cluster can go directly over the Azure VNet instead of through
the worker node's VXLAN tunnel. This builds on the existing
EXTERNAL_NETWORK_VIA_PODVMmechanism already used by the AWS and AlibabaCloud providers — see docs/azure-multi-nic.md
for configuration and design details.
AZURE_SECONDARY_SUBNET_IDconfig), since Azure doesn't support hot-attaching a NIC to a running VM
without first deallocating it, unlike AWS's post-launch ENI attach.
getIPsso the primary (control-plane) NIC'sIPs always come first, regardless of the order Azure returns NICs in.
CreateInstancefails fast with a clear error if multi-NIC pod networkingis requested (
EXTERNAL_NETWORK_VIA_PODVM=true) butAZURE_SECONDARY_SUBNET_IDisn't configured, instead of deferring to aruntime failure deep in pod network setup.
pkg/podnetwork's secondary-interface discovery (shared with theAWS/Alibaba providers) to infer a gateway when the secondary NIC has no
DHCP-assigned default route and isn't on the primary's subnet — the case
on Azure, since its two NICs sit on different subnets by design. This is
purely additive: existing discovery paths (own default route, or same
subnet as primary) are checked first and unchanged.
Test plan
go build/go vetclean forsrc/cloud-providers/azureandsrc/cloud-api-adaptor/pkg/podnetwork(Linux target)golangci-lint runclean on both changed packagesgo mod tidyproduces no diff in either modulesrc/cloud-providers/azure/multinic_test.go: NIC config building(single/multi-NIC, public IP placement),
orderByPrimaryIP ordering,ConfigVerifiersubnet validationsrc/cloud-api-adaptor/pkg/podnetwork/common_test.go: gatewayinference, cross-subnet/same-subnet secondary interface discovery,
end-to-end
setupExternalNetworkvia network namespaces (root-only,skipped locally on this dev machine — matches existing
podnetwork_test.goconvention, exercised by CI's
sudo make test)routing) — not exercised here after merging with latest code, the prototype was tested against 1.12 version of OSC on ARO.
DCO
This PR was co-developed with @devansh1109, who prototyped the original
implementation; I reworked it against current
main(which had divergedsignificantly — CSI volume support landed since the prototype was written),
removed debug-only logging, added the test coverage and doc above, and am
submitting it with their permission. Both commits are signed off per DCO 1.1.