From 96452969292e600e88dca30e33439c0ede1c4a30 Mon Sep 17 00:00:00 2001 From: Pooja V Date: Mon, 11 May 2026 18:02:42 +0530 Subject: [PATCH 1/3] Populate LoadBalancerBackendIPs with dual-stack SNAT IPs for NSX LB and AVI The LoadBalancerBackendIPs field was declared in VPCState but never populated. This change wires it up for all three LB paths: - NSX LB non-TEP-less: read all IPs from ExtendedAttributes["IpAddresses"].Values on the realized interface via new GetPolicyInterfaceIPs helper (both IPv4+IPv6). - NSX LB TEP-less: best-effort fetch of both _DEFAULT--VPC_SERVICE_IP (IPv4) and _DEFAULT--VPC_SERVICE_IP_V6 (IPv6) allocations; NotFound is silently skipped so VNA-only, VNS-IPv4-only, VNS-IPv6-only, and VNS-dual-stack all work correctly. - AVI: collect NetworkAddress from every VpcSubnetStatus result (one per IP family for dual-stack) instead of only Results[0]. LoadBalancerIPAddresses (existing field) is kept backward-compatible via a new primaryLBIP helper that follows the Kubernetes convention: IPv4 preferred for dual-stack, IPv6-only fallback, empty for no-LB. LoadBalancerBackendIPs is sorted before reflect.DeepEqual in setNetworkInfoVPCStatus to prevent spurious CR updates when IPs are returned in a different order. Unit tests added/updated for all modified functions across: pkg/nsx/services/realizestate, pkg/nsx/services/vpc, pkg/controllers/networkinfo (controller + utils). E2E tests added in test/e2e/nsx_networkinfo_test.go: testLBBackendIPsPopulated, testDualStackLBBackendIPs, testLBBackendIPsIdempotent. --- .../networkinfo/networkinfo_controller.go | 43 ++++-- .../networkinfo_controller_test.go | 123 ++++++++++++----- .../networkinfo/networkinfo_utils.go | 2 + .../networkinfo/networkinfo_utils_test.go | 42 ++++++ .../services/realizestate/realize_state.go | 29 ++++ .../realizestate/realize_state_test.go | 121 +++++++++++++++++ pkg/nsx/services/vpc/vpc.go | 100 +++++++++----- pkg/nsx/services/vpc/vpc_test.go | 109 ++++++++++++--- test/e2e/nsx_networkinfo_test.go | 125 ++++++++++++++++++ 9 files changed, 608 insertions(+), 86 deletions(-) diff --git a/pkg/controllers/networkinfo/networkinfo_controller.go b/pkg/controllers/networkinfo/networkinfo_controller.go index 76c0f5342..f322940bf 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller.go +++ b/pkg/controllers/networkinfo/networkinfo_controller.go @@ -6,6 +6,8 @@ package networkinfo import ( "context" "fmt" + "net" + "strings" "time" stderrors "github.com/vmware/vsphere-automation-sdk-go/lib/vapi/std/errors" @@ -319,7 +321,8 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } - snatIP, aviSubnetPath, aviSECIDR, nsxLBSNATIP, lbIP := "", "", "", "", "" + snatIP, aviSubnetPath, lbIP := "", "", "" + var lbBackendIPs []string var networkStack v1alpha1.NetworkStackType networkStack, err = r.Service.GetNetworkStackFromNC(nc) if err != nil { @@ -390,7 +393,8 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) // nsx bug, if set LoadBalancerVpcEndpoint.Enabled to false, when read this VPC back, // LoadBalancerVpcEndpoint.Enabled will become a nil pointer. if lbProvider == vpc.AVILB && createdVpc.LoadBalancerVpcEndpoint != nil && createdVpc.LoadBalancerVpcEndpoint.Enabled != nil && *createdVpc.LoadBalancerVpcEndpoint.Enabled { - aviSubnetPath, aviSECIDR, err = r.Service.GetAVISubnetInfo(*createdVpc) + var aviCIDRs []string + aviSubnetPath, aviCIDRs, err = r.Service.GetAVISubnetInfo(*createdVpc) if err != nil { log.Error(err, "Failed to read AVI LB Subnet path and CIDR", "VPC", createdVpc.Id) state := &v1alpha1.VPCState{ @@ -403,10 +407,12 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) setNSNetworkReadyCondition(ctx, r.Client, req.Namespace, nsMsgVPCAviSubnetError.getNSNetworkCondition(err)) return common.ResultRequeueAfter10sec, err } - lbIP = aviSECIDR + lbIP = primaryLBIP(aviCIDRs) + lbBackendIPs = aviCIDRs } else if lbProvider == vpc.NSXLB && len(nsxLBSPath) > 0 && len(vpcConnectivityProfilePath) > 0 { // Only check SNat IP when LB capability is ready and vpcConnectivityProfile exists. - nsxLBSNATIP, err = r.getNSXLBSNATIP(nc, createdVpc, vpcConnectivityProfilePath, gatewayConnectionReady, serviceClusterReady, networkStack == v1alpha1.VLANBackedVPC) + var nsxLBSNATIPs []string + nsxLBSNATIPs, err = r.getNSXLBSNATIP(nc, createdVpc, vpcConnectivityProfilePath, gatewayConnectionReady, serviceClusterReady, networkStack == v1alpha1.VLANBackedVPC) if err != nil { log.Error(err, "Failed to read NSX LB SNAT IP", "VPC", createdVpc.Id) state := &v1alpha1.VPCState{ @@ -419,13 +425,15 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) setNSNetworkReadyCondition(ctx, r.Client, req.Namespace, nsMsgVPCNSXLBSNATIPError.getNSNetworkCondition(err)) return common.ResultRequeueAfter10sec, err } - lbIP = nsxLBSNATIP + lbIP = primaryLBIP(nsxLBSNATIPs) + lbBackendIPs = nsxLBSNATIPs } state := &v1alpha1.VPCState{ Name: *createdVpc.DisplayName, DefaultSNATIP: snatIP, LoadBalancerIPAddresses: lbIP, + LoadBalancerBackendIPs: lbBackendIPs, PrivateIPs: privateIPs, NetworkStack: networkStack, } @@ -443,7 +451,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) return common.ResultNormal, nil } -func (r *NetworkInfoReconciler) getNSXLBSNATIP(nc *v1alpha1.VPCNetworkConfiguration, createdVpc *model.Vpc, vpcConnectivityProfilePath string, gatewayConnectionReady, serviceClusterReady bool, tepLess bool) (string, error) { +func (r *NetworkInfoReconciler) getNSXLBSNATIP(nc *v1alpha1.VPCNetworkConfiguration, createdVpc *model.Vpc, vpcConnectivityProfilePath string, gatewayConnectionReady, serviceClusterReady bool, tepLess bool) ([]string, error) { checkGatewayConnection := gatewayConnectionReady checkServiceCluster := serviceClusterReady // Precreated VPC uses different connectivity profile from system VPC @@ -451,7 +459,7 @@ func (r *NetworkInfoReconciler) getNSXLBSNATIP(nc *v1alpha1.VPCNetworkConfigurat if vpc.IsPreCreatedVPC(nc) { connectionStatus, err := r.Service.ValidateConnectionStatus(nc, vpcConnectivityProfilePath) if err != nil { - return "", err + return nil, err } checkGatewayConnection = connectionStatus.GatewayConnectionReady checkServiceCluster = connectionStatus.ServiceClusterReady @@ -463,7 +471,26 @@ func (r *NetworkInfoReconciler) getNSXLBSNATIP(nc *v1alpha1.VPCNetworkConfigurat // DTGW is used for NSX LB return r.Service.GetNSXLBSNATIP(*createdVpc, "service-interface", tepLess) } - return "", nil + return nil, nil +} + +// primaryLBIP selects the canonical single IP/CIDR for LoadBalancerIPAddresses following +// the Kubernetes dual-stack convention: prefer IPv4 for dual-stack, fall back to the first +// entry for IPv6-only, and return "" for an empty slice. +func primaryLBIP(ips []string) string { + for _, ip := range ips { + raw := ip + if idx := strings.Index(ip, "/"); idx != -1 { + raw = ip[:idx] + } + if net.ParseIP(raw) != nil && net.ParseIP(raw).To4() != nil { + return ip + } + } + if len(ips) > 0 { + return ips[0] + } + return "" } func (r *NetworkInfoReconciler) setupWithManager(mgr ctrl.Manager) error { diff --git a/pkg/controllers/networkinfo/networkinfo_controller_test.go b/pkg/controllers/networkinfo/networkinfo_controller_test.go index b2fbca6db..9d5eeb225 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller_test.go +++ b/pkg/controllers/networkinfo/networkinfo_controller_test.go @@ -217,8 +217,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultNSXLBSPathByVPC", func(_ *vpc.VPCService, _ string) string { return "lbs-path" @@ -289,8 +289,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultNSXLBSPathByVPC", func(_ *vpc.VPCService, _ string) string { return "lbs-path" @@ -362,8 +362,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultNSXLBSPathByVPC", func(_ *vpc.VPCService, _ string) string { return "lbs-path" @@ -469,8 +469,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "CreateOrUpdateVPC", func(_ *vpc.VPCService, ctx context.Context, _ *v1alpha1.NetworkInfo, _ *v1alpha1.VPCNetworkConfiguration, _ vpc.LBProvider) (*model.Vpc, error) { return &model.Vpc{ @@ -500,8 +500,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultSNATIP", func(_ *vpc.VPCService, _ model.Vpc) (string, error) { return "snat-ip", nil @@ -561,8 +561,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "CreateOrUpdateVPC", func(_ *vpc.VPCService, ctx context.Context, _ *v1alpha1.NetworkInfo, _ *v1alpha1.VPCNetworkConfiguration, _ vpc.LBProvider) (*model.Vpc, error) { return &model.Vpc{ @@ -643,8 +643,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "CreateOrUpdateVPC", func(_ *vpc.VPCService, ctx context.Context, _ *v1alpha1.NetworkInfo, _ *v1alpha1.VPCNetworkConfiguration, _ vpc.LBProvider) (*model.Vpc, error) { return &model.Vpc{ @@ -729,8 +729,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "CreateOrUpdateVPC", func(_ *vpc.VPCService, ctx context.Context, _ *v1alpha1.NetworkInfo, _ *v1alpha1.VPCNetworkConfiguration, _ vpc.LBProvider) (*model.Vpc, error) { return &model.Vpc{ @@ -757,8 +757,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultNSXLBSPathByVPC", func(_ *vpc.VPCService, _ string) string { return "lbs-path" }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetDefaultSNATIP", func(_ *vpc.VPCService, _ model.Vpc) (string, error) { return "snat-ip", nil @@ -816,8 +816,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.Service), "GetLBProvider", func(_ *vpc.VPCService) (vpc.LBProvider, error) { return vpc.NSXLB, nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "100.64.0.3", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "CreateOrUpdateVPC", func(_ *vpc.VPCService, ctx context.Context, _ *v1alpha1.NetworkInfo, _ *v1alpha1.VPCNetworkConfiguration, _ vpc.LBProvider) (*model.Vpc, error) { return &model.Vpc{ @@ -987,9 +987,9 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { }, nil}, Times: 2, }}) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, interfaceID string) (string, error) { + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, interfaceID string) ([]string, error) { assert.Equal(t, "gateway-interface", interfaceID) - return "100.64.0.3", nil + return []string{"100.64.0.3"}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil @@ -1111,9 +1111,9 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { }, nil }) // GetAVISubnetInfo should be called even without connectivity profile - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetAVISubnetInfo", func(_ *vpc.VPCService, _ model.Vpc) (string, string, error) { - return "/orgs/default/projects/project-quality/vpcs/fake-vpc/subnets/avi-subnet", "100.64.0.0/24", nil - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetAVISubnetInfo", func(_ *vpc.VPCService, _ model.Vpc) (string, []string, error) { + return "/orgs/default/projects/project-quality/vpcs/fake-vpc/subnets/avi-subnet", []string{"100.64.0.0/24"}, nil + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) @@ -1183,10 +1183,10 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { return "", nil }) // This should NOT be called when vpcConnectivityProfilePath is empty - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - assert.FailNow(t, "GetNSXLBSNATIP should not be called when vpcConnectivityProfilePath is empty") - return "", nil - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + assert.FailNow(t, "GetNSXLBSNATIP should not be called when vpcConnectivityProfilePath is empty") + return nil, nil + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) @@ -1275,9 +1275,9 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { assert.FailNow(t, "should set VPCNetworkConfiguration status with AutoSnatEnabled=false") } }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) (string, error) { - return "", fmt.Errorf("tier1 uplink port IP not found") - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return nil, fmt.Errorf("tier1 uplink port IP not found") + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) @@ -2298,3 +2298,62 @@ func (m *MockManager) Add(runnable manager.Runnable) error { func (m *MockManager) Start(context.Context) error { return nil } + +func TestPrimaryLBIP(t *testing.T) { + tests := []struct { + name string + ips []string + want string + }{ + { + name: "nil slice returns empty string", + ips: nil, + want: "", + }, + { + name: "empty slice returns empty string", + ips: []string{}, + want: "", + }, + { + name: "single IPv4 bare IP", + ips: []string{"100.64.0.1"}, + want: "100.64.0.1", + }, + { + name: "single IPv6 bare IP falls back to first", + ips: []string{"2001:db8::1"}, + want: "2001:db8::1", + }, + { + name: "dual-stack: IPv4 preferred (bare IPs)", + ips: []string{"100.64.0.1", "2001:db8::1"}, + want: "100.64.0.1", + }, + { + name: "dual-stack: IPv4 preferred even when IPv6 is first", + ips: []string{"2001:db8::1", "100.64.0.1"}, + want: "100.64.0.1", + }, + { + name: "single IPv4 CIDR (AVI format)", + ips: []string{"100.64.0.0/24"}, + want: "100.64.0.0/24", + }, + { + name: "single IPv6 CIDR falls back to first", + ips: []string{"2001:db8::/64"}, + want: "2001:db8::/64", + }, + { + name: "dual-stack CIDR: IPv4 preferred", + ips: []string{"100.64.0.0/24", "2001:db8::/64"}, + want: "100.64.0.0/24", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, primaryLBIP(tt.ips)) + }) + } +} diff --git a/pkg/controllers/networkinfo/networkinfo_utils.go b/pkg/controllers/networkinfo/networkinfo_utils.go index 4ea3f1e46..5871b23a3 100644 --- a/pkg/controllers/networkinfo/networkinfo_utils.go +++ b/pkg/controllers/networkinfo/networkinfo_utils.go @@ -40,6 +40,8 @@ func setNetworkInfoVPCStatus(client client.Client, ctx context.Context, obj clie } slices.Sort(existingVPC.PrivateIPs) slices.Sort(createdVPC.PrivateIPs) + slices.Sort(existingVPC.LoadBalancerBackendIPs) + slices.Sort(createdVPC.LoadBalancerBackendIPs) if reflect.DeepEqual(*existingVPC, *createdVPC) { return } diff --git a/pkg/controllers/networkinfo/networkinfo_utils_test.go b/pkg/controllers/networkinfo/networkinfo_utils_test.go index 7a845ca8b..5efef0a8f 100644 --- a/pkg/controllers/networkinfo/networkinfo_utils_test.go +++ b/pkg/controllers/networkinfo/networkinfo_utils_test.go @@ -413,6 +413,48 @@ func TestGetNSNetworkCondition(t *testing.T) { require.True(t, nsConditionEquals(vpcNotReadyCondition, *nsMsgVPCCreateUpdateError.getNSNetworkCondition(msgErr))) } +// TestSetNetworkInfoVPCStatusIdempotency verifies that reordered LoadBalancerBackendIPs do +// not trigger a spurious CR update because setNetworkInfoVPCStatus sorts both slices before +// reflect.DeepEqual. +func TestSetNetworkInfoVPCStatusIdempotency(t *testing.T) { + ctx := context.TODO() + scheme := clientgoscheme.Scheme + v1alpha1.AddToScheme(scheme) + + // Build a NetworkInfo CR that already has a VPC state with LoadBalancerBackendIPs. + existingNetworkInfo := &v1alpha1.NetworkInfo{ + ObjectMeta: metav1.ObjectMeta{Name: "ni", Namespace: "default"}, + VPCs: []v1alpha1.VPCState{ + { + Name: "vpc1", + LoadBalancerIPAddresses: "100.64.0.1", + LoadBalancerBackendIPs: []string{"100.64.0.1", "2001:db8::1"}, + }, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingNetworkInfo).Build() + + // The reconcile produces the same state but with IPs in reverse order. + reorderedVPC := &v1alpha1.VPCState{ + Name: "vpc1", + LoadBalancerIPAddresses: "100.64.0.1", + LoadBalancerBackendIPs: []string{"2001:db8::1", "100.64.0.1"}, + } + + // setNetworkInfoVPCStatus must NOT call fakeClient.Update because the states are equal + // after sorting. We capture any update by reading the CR back; ResourceVersion should + // be unchanged (fake client does not increment it on no-op, but the Update itself should + // not be called when the sorted slices are equal). + originalRV := existingNetworkInfo.ResourceVersion + + setNetworkInfoVPCStatus(fakeClient, ctx, existingNetworkInfo, metav1.Time{}, reorderedVPC) + + // Reload the CR and confirm no actual write occurred. + refreshed := &v1alpha1.NetworkInfo{} + require.NoError(t, fakeClient.Get(ctx, apitypes.NamespacedName{Name: "ni", Namespace: "default"}, refreshed)) + assert.Equal(t, originalRV, refreshed.ResourceVersion, "CR should not be updated when only IP order differs") +} + func TestHasPodOrVMDefaultSubnets(t *testing.T) { subnets := []v1alpha1.SharedSubnet{ { diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go index 474115149..4b2b4f755 100644 --- a/pkg/nsx/services/realizestate/realize_state.go +++ b/pkg/nsx/services/realizestate/realize_state.go @@ -108,3 +108,32 @@ func (service *RealizeStateService) GetPolicyInterfaceIP(realizedPath string) (s return "", fmt.Errorf("%s LB SNAT IP not found", realizedPath) } + +// GetPolicyInterfaceIPs returns all bare IPs (with CIDR prefix stripped) from the +// IpAddresses extended attribute on the realized entity. For dual-stack interfaces both +// the IPv4 and IPv6 entries are returned. +func (service *RealizeStateService) GetPolicyInterfaceIPs(realizedPath string) ([]string, error) { + result, err := service.NSXClient.RealizedEntityClient.Get(realizedPath) + err = nsxutil.TransNSXApiError(err) + if err != nil { + return nil, err + } + + var ips []string + for i := range result.ExtendedAttributes { + if result.ExtendedAttributes[i].Key != nil && *result.ExtendedAttributes[i].Key == "IpAddresses" { + for _, ip := range result.ExtendedAttributes[i].Values { + parts := strings.Split(ip, "/") + if len(parts) != 2 { + continue + } + ips = append(ips, parts[0]) + } + } + } + + if len(ips) == 0 { + return nil, fmt.Errorf("%s LB SNAT IP not found", realizedPath) + } + return ips, nil +} diff --git a/pkg/nsx/services/realizestate/realize_state_test.go b/pkg/nsx/services/realizestate/realize_state_test.go index da25ace86..20e347cca 100644 --- a/pkg/nsx/services/realizestate/realize_state_test.go +++ b/pkg/nsx/services/realizestate/realize_state_test.go @@ -1,11 +1,13 @@ package realizestate import ( + "fmt" "testing" "time" "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" "k8s.io/apimachinery/pkg/util/wait" @@ -18,12 +20,29 @@ import ( type fakeRealizedEntitiesClient struct{} +// fakeRealizedEntityClient implements infra_realized.RealizedEntityClient. +// The getFn callback allows each test case to control the response without gomonkey. +type fakeRealizedEntityClient struct { + getFn func(realizedPathParam string) (model.GenericPolicyRealizedResource, error) +} + func (c *fakeRealizedEntitiesClient) List(intentPathParam string, sitePathParam *string) (model.GenericPolicyRealizedResourceListResult, error) { return model.GenericPolicyRealizedResourceListResult{ Results: []model.GenericPolicyRealizedResource{}, }, nil } +func (c *fakeRealizedEntityClient) Get(realizedPathParam string) (model.GenericPolicyRealizedResource, error) { + if c.getFn != nil { + return c.getFn(realizedPathParam) + } + return model.GenericPolicyRealizedResource{}, nil +} + +func (c *fakeRealizedEntityClient) Refresh(intentPathParam string, enforcementPointPathParam *string) error { + return nil +} + func TestRealizeStateService_CheckRealizeState(t *testing.T) { commonService := common.Service{ NSXClient: &nsx.Client{ @@ -303,3 +322,105 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) { patches.Reset() } + +func TestGetPolicyInterfaceIPs(t *testing.T) { + realizedPath := "/orgs/default/projects/default/realized-state/enforcement-points/default/vpcs/vpc1/gateway-interface" + + tests := []struct { + name string + getFn func(realizedPathParam string) (model.GenericPolicyRealizedResource, error) + wantIPs []string + wantErr string + }{ + { + name: "single IPv4", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + key := "IpAddresses" + return model.GenericPolicyRealizedResource{ + ExtendedAttributes: []model.AttributeVal{ + {Key: &key, Values: []string{"100.64.0.1/29"}}, + }, + }, nil + }, + wantIPs: []string{"100.64.0.1"}, + }, + { + name: "dual-stack IPv4 and IPv6", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + key := "IpAddresses" + return model.GenericPolicyRealizedResource{ + ExtendedAttributes: []model.AttributeVal{ + {Key: &key, Values: []string{"100.64.0.1/29", "2001:db8::1/64"}}, + }, + }, nil + }, + wantIPs: []string{"100.64.0.1", "2001:db8::1"}, + }, + { + name: "IPv6-only", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + key := "IpAddresses" + return model.GenericPolicyRealizedResource{ + ExtendedAttributes: []model.AttributeVal{ + {Key: &key, Values: []string{"2001:db8::1/64"}}, + }, + }, nil + }, + wantIPs: []string{"2001:db8::1"}, + }, + { + name: "empty Values returns error", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + key := "IpAddresses" + return model.GenericPolicyRealizedResource{ + ExtendedAttributes: []model.AttributeVal{ + {Key: &key, Values: []string{}}, + }, + }, nil + }, + wantErr: "LB SNAT IP not found", + }, + { + name: "missing IpAddresses key returns error", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + return model.GenericPolicyRealizedResource{}, nil + }, + wantErr: "LB SNAT IP not found", + }, + { + name: "Get returns error", + getFn: func(_ string) (model.GenericPolicyRealizedResource, error) { + return model.GenericPolicyRealizedResource{}, fmt.Errorf("connection refused") + }, + wantErr: "connection refused", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeEntityClient := &fakeRealizedEntityClient{getFn: tt.getFn} + service := &RealizeStateService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + RealizedEntityClient: fakeEntityClient, + NsxConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{Cluster: "k8scl-one:test"}, + }, + }, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{Cluster: "k8scl-one:test"}, + }, + }, + } + + ips, err := service.GetPolicyInterfaceIPs(realizedPath) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantIPs, ips) + }) + } +} diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go index d566959a8..7509b129c 100644 --- a/pkg/nsx/services/vpc/vpc.go +++ b/pkg/nsx/services/vpc/vpc.go @@ -31,7 +31,8 @@ const ( NSXLB = LBProvider("nsx-lb") AVILB = LBProvider("avi") NoneLB = LBProvider("none") - NSXLBEIPID = "_DEFAULT--VPC_SERVICE_IP" + NSXLBEIPID = "_DEFAULT--VPC_SERVICE_IP" + NSXLBEIPIDV6 = "_DEFAULT--VPC_SERVICE_IP_V6" nsxVpcNameIndexKey = "nsxVpcNameIndex" @@ -363,19 +364,19 @@ func (s *VPCService) GetDefaultSNATIP(vpc model.Vpc) (string, error) { return *results.Results[0].TranslatedNetwork, nil } -func (s *VPCService) GetAVISubnetInfo(vpc model.Vpc) (string, string, error) { +func (s *VPCService) GetAVISubnetInfo(vpc model.Vpc) (string, []string, error) { subnetsClient := s.NSXClient.SubnetsClient statusClient := s.NSXClient.SubnetStatusClient info, err := common.ParseVPCResourcePath(*vpc.Path) if err != nil { - return "", "", err + return "", nil, err } subnet, err := subnetsClient.Get(info.OrgID, info.ProjectID, info.VPCID, common.AVISubnetLBID) err = nsxutil.TransNSXApiError(err) if err != nil { log.Error(err, "Failed to read AVI subnet", "VPC", vpc.Id) - return "", "", err + return "", nil, err } path := *subnet.Path @@ -383,61 +384,100 @@ func (s *VPCService) GetAVISubnetInfo(vpc model.Vpc) (string, string, error) { err = nsxutil.TransNSXApiError(err) if err != nil { log.Error(err, "Failed to read AVI subnet status", "VPC", vpc.Id) - return "", "", err + return "", nil, err } if len(statusList.Results) == 0 { log.Info("AVI subnet status not found", "VPC", vpc.Id) - return "", "", err + return "", nil, fmt.Errorf("AVI subnet status not found for VPC %s", *vpc.Id) } - if statusList.Results[0].NetworkAddress == nil { - err := fmt.Errorf("invalid status result: %+v", statusList.Results[0]) + var cidrs []string + for i := range statusList.Results { + if statusList.Results[i].NetworkAddress == nil { + log.Info("Skipping AVI subnet status result with nil NetworkAddress", "Subnet", common.AVISubnetLBID, "index", i) + continue + } + cidrs = append(cidrs, *statusList.Results[i].NetworkAddress) + } + + if len(cidrs) == 0 { + err := fmt.Errorf("no network address found in AVI subnet status for VPC %s", *vpc.Id) log.Error(err, "Subnet status does not have network address", "Subnet", common.AVISubnetLBID) - return "", "", err + return "", nil, err } - cidr := *statusList.Results[0].NetworkAddress - log.Info("Read AVI subnet properties", "Path", path, "CIDR", cidr) - return path, cidr, nil + log.Info("Read AVI subnet properties", "Path", path, "CIDRs", cidrs) + return path, cidrs, nil } -func (s *VPCService) GetNSXLBSNATIPForTepLessVPC(vpcPath string) (string, error) { +// GetNSXLBSNATIPsForTepLessVPC fetches the VPC service IPs for a TEP-less VPC. +// IPv4 and IPv6 allocations are fetched independently (best-effort): a Not-Found response +// for either allocation ID is silently skipped because which allocations exist depends on +// the node type (VNA vs VNS) and the IP blocks configured in the VPC connectivity profile. +// An error is returned only when both fetches yield no IP. +func (s *VPCService) GetNSXLBSNATIPsForTepLessVPC(vpcPath string) ([]string, error) { pathInfo, err := common.ParseVPCResourcePath(vpcPath) if err != nil { - return "", err + return nil, err } - log.Info("Getting VPC NSX LB SNAT IP", "Path", vpcPath) - ipaddressallocation, err := s.NSXClient.IPAddressAllocationClient.Get(pathInfo.OrgID, pathInfo.ProjectID, pathInfo.VPCID, NSXLBEIPID) + log.Info("Getting VPC NSX LB SNAT IPs for TEP-less VPC", "Path", vpcPath) + + var ips []string + + v4, err := s.NSXClient.IPAddressAllocationClient.Get(pathInfo.OrgID, pathInfo.ProjectID, pathInfo.VPCID, NSXLBEIPID) + err = nsxutil.TransNSXApiError(err) if err != nil { - log.Error(err, "Failed to get NSX LB SNAT IP for TEP-less VPC", "VPC", pathInfo.VPCID) - return "", err + if nsxErr, ok := err.(*nsxutil.NSXApiError); !ok || nsxErr.Type() != stderrors.ErrorType_NOT_FOUND { + log.Error(err, "Failed to get NSX LB IPv4 SNAT IP for TEP-less VPC", "VPC", pathInfo.VPCID) + return nil, err + } + log.Info("NSX LB IPv4 SNAT IP allocation not found, skipping", "VPC", pathInfo.VPCID) + } else if v4.AllocationIps != nil { + ips = append(ips, *v4.AllocationIps) + log.Info("Got NSX LB IPv4 SNAT IP", "VPC", pathInfo.VPCID, "IP", *v4.AllocationIps) } - log.Info("Getting VPC NSX LB SNAT IP", "VPC", pathInfo.VPCID, "SNATIP", *ipaddressallocation.AllocationIps) - return *ipaddressallocation.AllocationIps, nil + + v6, err := s.NSXClient.IPAddressAllocationClient.Get(pathInfo.OrgID, pathInfo.ProjectID, pathInfo.VPCID, NSXLBEIPIDV6) + err = nsxutil.TransNSXApiError(err) + if err != nil { + if nsxErr, ok := err.(*nsxutil.NSXApiError); !ok || nsxErr.Type() != stderrors.ErrorType_NOT_FOUND { + log.Error(err, "Failed to get NSX LB IPv6 SNAT IP for TEP-less VPC", "VPC", pathInfo.VPCID) + return nil, err + } + log.Info("NSX LB IPv6 SNAT IP allocation not found, skipping", "VPC", pathInfo.VPCID) + } else if v6.AllocationIps != nil { + ips = append(ips, *v6.AllocationIps) + log.Info("Got NSX LB IPv6 SNAT IP", "VPC", pathInfo.VPCID, "IP", *v6.AllocationIps) + } + + if len(ips) == 0 { + return nil, fmt.Errorf("no VPC service IP found for TEP-less VPC %s", pathInfo.VPCID) + } + return ips, nil } -func (s *VPCService) GetNSXLBSNATIP(vpc model.Vpc, interfaceID string, tepLess bool) (string, error) { - log.Info("Getting VPC NSX LB SNAT IP", "VPC", *vpc.Id) +func (s *VPCService) GetNSXLBSNATIP(vpc model.Vpc, interfaceID string, tepLess bool) ([]string, error) { + log.Info("Getting VPC NSX LB SNAT IPs", "VPC", *vpc.Id) if tepLess { - return s.GetNSXLBSNATIPForTepLessVPC(*vpc.Path) + return s.GetNSXLBSNATIPsForTepLessVPC(*vpc.Path) } vpcInfo, err := common.ParseVPCResourcePath(*vpc.Path) if err != nil { - return "", err + return nil, err } realizeService := realizestate.InitializeRealizeState(s.Service) realizedPath := fmt.Sprintf("/orgs/%s/projects/%s/realized-state/enforcement-points/default/vpcs/%s/%s", vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, interfaceID) - log.Info("Getting VPC NSX LB SNAT IP", "Path", realizedPath) - lbIP, err := realizeService.GetPolicyInterfaceIP(realizedPath) + log.Info("Getting VPC NSX LB SNAT IPs", "Path", realizedPath) + lbIPs, err := realizeService.GetPolicyInterfaceIPs(realizedPath) if err != nil { - log.Error(err, "Failed to get VPC NSX LB SNAT IP", "VPC", *vpc.Id) - return "", err + log.Error(err, "Failed to get VPC NSX LB SNAT IPs", "VPC", *vpc.Id) + return nil, err } - log.Info("Getting VPC NSX LB SNAT IP", "VPC", *vpc.Id, "SNATIP", lbIP) - return lbIP, nil + log.Info("Getting VPC NSX LB SNAT IPs", "VPC", *vpc.Id, "SNATIPs", lbIPs) + return lbIPs, nil } func (s *VPCService) GetVpcConnectivityProfile(nc *v1alpha1.VPCNetworkConfiguration, vpcConnectivityProfilePath string) (*model.VpcConnectivityProfile, error) { diff --git a/pkg/nsx/services/vpc/vpc_test.go b/pkg/nsx/services/vpc/vpc_test.go index 6f9de7b1a..f859d1b36 100644 --- a/pkg/nsx/services/vpc/vpc_test.go +++ b/pkg/nsx/services/vpc/vpc_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vmware/vsphere-automation-sdk-go/runtime/data" + stderrors "github.com/vmware/vsphere-automation-sdk-go/lib/vapi/std/errors" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" @@ -2777,50 +2778,126 @@ func TestGetNSXLBSNATIP(t *testing.T) { vpc model.Vpc isTepLess bool prepareFuncs func() *gomonkey.Patches - wantObj string + wantObj []string wantErr string }{ { - name: "Test normal case", + name: "non-TEP-less IPv4-only", vpc: vpc1, - wantObj: "100.64.0.3", + wantObj: []string{"100.64.0.3"}, prepareFuncs: func() *gomonkey.Patches { - patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIP, - func(_ *realizestate.RealizeStateService, _ string) (string, error) { - return "100.64.0.3", nil + patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIPs, + func(_ *realizestate.RealizeStateService, _ string) ([]string, error) { + return []string{"100.64.0.3"}, nil }) return patches }, }, { - name: "nsx lb uplink port IP not found error", + name: "non-TEP-less dual-stack", + vpc: vpc1, + wantObj: []string{"100.64.0.3", "2001:db8::1"}, + prepareFuncs: func() *gomonkey.Patches { + patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIPs, + func(_ *realizestate.RealizeStateService, _ string) ([]string, error) { + return []string{"100.64.0.3", "2001:db8::1"}, nil + }) + return patches + }, + }, + { + name: "non-TEP-less IPv6-only", + vpc: vpc1, + wantObj: []string{"2001:db8::1"}, + prepareFuncs: func() *gomonkey.Patches { + patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIPs, + func(_ *realizestate.RealizeStateService, _ string) ([]string, error) { + return []string{"2001:db8::1"}, nil + }) + return patches + }, + }, + { + name: "non-TEP-less uplink port IP not found error", vpc: vpc1, prepareFuncs: func() *gomonkey.Patches { - patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIP, - func(_ *realizestate.RealizeStateService, _ string) (string, error) { - return "", fmt.Errorf("fake-vpc tier1 uplink port IP not found") + patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).GetPolicyInterfaceIPs, + func(_ *realizestate.RealizeStateService, _ string) ([]string, error) { + return nil, fmt.Errorf("fake-vpc tier1 uplink port IP not found") }) return patches }, wantErr: "tier1 uplink port IP not found", }, { - name: "TEP-less VPC success", + name: "TEP-less VPC IPv4-only (IPv6 allocation not found)", vpc: vpc1, isTepLess: true, + wantObj: []string{"100.64.0.5"}, prepareFuncs: func() *gomonkey.Patches { + callCount := 0 patches := gomonkey.ApplyMethod(reflect.TypeOf(&fakeIPAddressAllocationClientInstance), "Get", func(_ *fakeIPAddressAllocationClient, _, _, _ string) (model.VpcIpAddressAllocation, error) { - return model.VpcIpAddressAllocation{ - AllocationIps: ptr.To("100.64.0.5"), - }, nil + callCount++ + if callCount == 1 { + return model.VpcIpAddressAllocation{AllocationIps: ptr.To("100.64.0.5")}, nil + } + return model.VpcIpAddressAllocation{}, nsxUtil.NewNSXApiError(nil, stderrors.ErrorType_NOT_FOUND) + }) + return patches + }, + }, + { + name: "TEP-less VPC dual-stack (IPv4 and IPv6)", + vpc: vpc1, + isTepLess: true, + wantObj: []string{"100.64.0.5", "2001:db8::5"}, + prepareFuncs: func() *gomonkey.Patches { + callCount := 0 + patches := gomonkey.ApplyMethod(reflect.TypeOf(&fakeIPAddressAllocationClientInstance), "Get", + func(_ *fakeIPAddressAllocationClient, _, _, _ string) (model.VpcIpAddressAllocation, error) { + callCount++ + if callCount == 1 { + return model.VpcIpAddressAllocation{AllocationIps: ptr.To("100.64.0.5")}, nil + } + return model.VpcIpAddressAllocation{AllocationIps: ptr.To("2001:db8::5")}, nil + }) + return patches + }, + }, + { + name: "TEP-less VPC IPv6-only (IPv4 allocation not found)", + vpc: vpc1, + isTepLess: true, + wantObj: []string{"2001:db8::5"}, + prepareFuncs: func() *gomonkey.Patches { + callCount := 0 + patches := gomonkey.ApplyMethod(reflect.TypeOf(&fakeIPAddressAllocationClientInstance), "Get", + func(_ *fakeIPAddressAllocationClient, _, _, _ string) (model.VpcIpAddressAllocation, error) { + callCount++ + if callCount == 1 { + return model.VpcIpAddressAllocation{}, nsxUtil.NewNSXApiError(nil, stderrors.ErrorType_NOT_FOUND) + } + return model.VpcIpAddressAllocation{AllocationIps: ptr.To("2001:db8::5")}, nil + }) + return patches + }, + }, + { + name: "TEP-less VPC both allocations not found", + vpc: vpc1, + isTepLess: true, + prepareFuncs: func() *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(&fakeIPAddressAllocationClientInstance), "Get", + func(_ *fakeIPAddressAllocationClient, _, _, _ string) (model.VpcIpAddressAllocation, error) { + return model.VpcIpAddressAllocation{}, nsxUtil.NewNSXApiError(nil, stderrors.ErrorType_NOT_FOUND) }) return patches }, - wantObj: "100.64.0.5", + wantErr: "no VPC service IP found for TEP-less VPC", }, { - name: "TEP-less VPC IPAddressAllocation error", + name: "TEP-less VPC IPAddressAllocation hard error", vpc: vpc1, isTepLess: true, prepareFuncs: func() *gomonkey.Patches { diff --git a/test/e2e/nsx_networkinfo_test.go b/test/e2e/nsx_networkinfo_test.go index 584854be0..7faa0573f 100644 --- a/test/e2e/nsx_networkinfo_test.go +++ b/test/e2e/nsx_networkinfo_test.go @@ -3,6 +3,8 @@ package e2e import ( "context" "fmt" + "net" + "strings" "testing" "time" @@ -58,6 +60,18 @@ func TestNetworkInfo(t *testing.T) { StartParallel(t) testDefaultNetworkInfo(t) }) + RunSubtest(t, "testLBBackendIPsPopulated", func(t *testing.T) { + StartParallel(t) + testLBBackendIPsPopulated(t) + }) + RunSubtest(t, "testDualStackLBBackendIPs", func(t *testing.T) { + StartParallel(t) + testDualStackLBBackendIPs(t) + }) + RunSubtest(t, "testLBBackendIPsIdempotent", func(t *testing.T) { + StartParallel(t) + testLBBackendIPsIdempotent(t) + }) }) // SequentialTests: These tests share testCustomizedNetworkConfigName (VPCNetworkConfiguration) @@ -456,3 +470,114 @@ func deleteVPCNetworkConfiguration(t *testing.T, ncName string) { }) require.NoError(t, err) } + +// testLBBackendIPsPopulated verifies that LoadBalancerBackendIPs is populated for a +// VPC with LB configured, and that every entry is a valid IP or CIDR. It also confirms +// that LoadBalancerIPAddresses (the primary field) appears in the backend list. +func testLBBackendIPsPopulated(t *testing.T) { + assureNamespace(t, infraVPCNamespace) + + networkInfo := getNetworkInfoWithCondition(t, infraVPCNamespace, infraVPCNamespace, + func(ni *v1alpha1.NetworkInfo) (bool, error) { + if len(ni.VPCs) == 0 { + return false, nil + } + return len(ni.VPCs[0].LoadBalancerBackendIPs) > 0, nil + }) + + require.NotNil(t, networkInfo) + require.NotEmpty(t, networkInfo.VPCs) + vpc := networkInfo.VPCs[0] + + if vpc.LoadBalancerIPAddresses != "" { + assert.Contains(t, vpc.LoadBalancerBackendIPs, vpc.LoadBalancerIPAddresses, + "LoadBalancerIPAddresses must appear in LoadBalancerBackendIPs") + } + + for _, entry := range vpc.LoadBalancerBackendIPs { + raw := entry + if i := strings.Index(entry, "/"); i >= 0 { + raw = entry[:i] + } + assert.NotNil(t, net.ParseIP(raw), + "LoadBalancerBackendIPs entry %q is not a valid IP or CIDR", entry) + } +} + +// testDualStackLBBackendIPs verifies that on a dual-stack cluster the backend list +// contains at least one IPv4 and one IPv6 entry, and that LoadBalancerIPAddresses +// (the primary field) is the IPv4 entry per the k8s convention. +func testDualStackLBBackendIPs(t *testing.T) { + if clusterInfo.podV6NetworkCIDR == "" { + t.Skip("cluster does not support IPv6; skipping dual-stack LB backend IPs test") + } + + assureNamespace(t, infraVPCNamespace) + + networkInfo := getNetworkInfoWithCondition(t, infraVPCNamespace, infraVPCNamespace, + func(ni *v1alpha1.NetworkInfo) (bool, error) { + if len(ni.VPCs) == 0 { + return false, nil + } + return len(ni.VPCs[0].LoadBalancerBackendIPs) >= 2, nil + }) + + require.NotNil(t, networkInfo) + require.NotEmpty(t, networkInfo.VPCs) + vpc := networkInfo.VPCs[0] + + var hasIPv4, hasIPv6 bool + for _, entry := range vpc.LoadBalancerBackendIPs { + raw := entry + if i := strings.Index(entry, "/"); i >= 0 { + raw = entry[:i] + } + ip := net.ParseIP(raw) + require.NotNil(t, ip, "invalid IP/CIDR in LoadBalancerBackendIPs: %q", entry) + if ip.To4() != nil { + hasIPv4 = true + } else { + hasIPv6 = true + } + } + assert.True(t, hasIPv4, "expected at least one IPv4 entry in LoadBalancerBackendIPs") + assert.True(t, hasIPv6, "expected at least one IPv6 entry in LoadBalancerBackendIPs") + + if vpc.LoadBalancerIPAddresses != "" { + raw := vpc.LoadBalancerIPAddresses + if i := strings.Index(raw, "/"); i >= 0 { + raw = raw[:i] + } + ip := net.ParseIP(raw) + require.NotNil(t, ip, "LoadBalancerIPAddresses is not a valid IP/CIDR: %q", vpc.LoadBalancerIPAddresses) + assert.NotNil(t, ip.To4(), + "LoadBalancerIPAddresses should be IPv4 on a dual-stack cluster (k8s convention)") + } +} + +// testLBBackendIPsIdempotent verifies that successive reconcile loops do not produce +// spurious CR updates once LoadBalancerBackendIPs is already correctly populated. +func testLBBackendIPsIdempotent(t *testing.T) { + assureNamespace(t, infraVPCNamespace) + + networkInfo := getNetworkInfoWithCondition(t, infraVPCNamespace, infraVPCNamespace, + func(ni *v1alpha1.NetworkInfo) (bool, error) { + if len(ni.VPCs) == 0 { + return false, nil + } + return len(ni.VPCs[0].LoadBalancerBackendIPs) > 0, nil + }) + + require.NotNil(t, networkInfo) + baseRV := networkInfo.ResourceVersion + require.NotEmpty(t, baseRV, "expected non-empty ResourceVersion after initial reconcile") + + // Allow time for additional reconcile loops to run + time.Sleep(30 * time.Second) + + updated, err := testData.crdClientset.CrdV1alpha1().NetworkInfos(infraVPCNamespace).Get( + context.Background(), infraVPCNamespace, v1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, baseRV, updated.ResourceVersion, + "ResourceVersion changed after idle period — spurious update detected in LoadBalancerBackendIPs reconcile") +} From a5ec5286ac50e98f5a764606923fcad12118851e Mon Sep 17 00:00:00 2001 From: Pooja V Date: Thu, 14 May 2026 18:42:24 +0530 Subject: [PATCH 2/3] fix lint issue --- .../networkinfo_controller_test.go | 20 +++++++++---------- pkg/nsx/services/vpc/vpc.go | 4 ++-- pkg/nsx/services/vpc/vpc_test.go | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/controllers/networkinfo/networkinfo_controller_test.go b/pkg/controllers/networkinfo/networkinfo_controller_test.go index 9d5eeb225..758ebea00 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller_test.go +++ b/pkg/controllers/networkinfo/networkinfo_controller_test.go @@ -1111,9 +1111,9 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { }, nil }) // GetAVISubnetInfo should be called even without connectivity profile - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetAVISubnetInfo", func(_ *vpc.VPCService, _ model.Vpc) (string, []string, error) { - return "/orgs/default/projects/project-quality/vpcs/fake-vpc/subnets/avi-subnet", []string{"100.64.0.0/24"}, nil - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetAVISubnetInfo", func(_ *vpc.VPCService, _ model.Vpc) (string, []string, error) { + return "/orgs/default/projects/project-quality/vpcs/fake-vpc/subnets/avi-subnet", []string{"100.64.0.0/24"}, nil + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) @@ -1183,10 +1183,10 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { return "", nil }) // This should NOT be called when vpcConnectivityProfilePath is empty - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { - assert.FailNow(t, "GetNSXLBSNATIP should not be called when vpcConnectivityProfilePath is empty") - return nil, nil - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + assert.FailNow(t, "GetNSXLBSNATIP should not be called when vpcConnectivityProfilePath is empty") + return nil, nil + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) @@ -1275,9 +1275,9 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { assert.FailNow(t, "should set VPCNetworkConfiguration status with AutoSnatEnabled=false") } }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { - return nil, fmt.Errorf("tier1 uplink port IP not found") - }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNSXLBSNATIP", func(_ *vpc.VPCService, _ model.Vpc, _ string) ([]string, error) { + return nil, fmt.Errorf("tier1 uplink port IP not found") + }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil }) diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go index 7509b129c..29dbd4d70 100644 --- a/pkg/nsx/services/vpc/vpc.go +++ b/pkg/nsx/services/vpc/vpc.go @@ -31,8 +31,8 @@ const ( NSXLB = LBProvider("nsx-lb") AVILB = LBProvider("avi") NoneLB = LBProvider("none") - NSXLBEIPID = "_DEFAULT--VPC_SERVICE_IP" - NSXLBEIPIDV6 = "_DEFAULT--VPC_SERVICE_IP_V6" + NSXLBEIPID = "_DEFAULT--VPC_SERVICE_IP" + NSXLBEIPIDV6 = "_DEFAULT--VPC_SERVICE_IP_V6" nsxVpcNameIndexKey = "nsxVpcNameIndex" diff --git a/pkg/nsx/services/vpc/vpc_test.go b/pkg/nsx/services/vpc/vpc_test.go index f859d1b36..952eb03ca 100644 --- a/pkg/nsx/services/vpc/vpc_test.go +++ b/pkg/nsx/services/vpc/vpc_test.go @@ -12,8 +12,8 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/vmware/vsphere-automation-sdk-go/runtime/data" stderrors "github.com/vmware/vsphere-automation-sdk-go/lib/vapi/std/errors" + "github.com/vmware/vsphere-automation-sdk-go/runtime/data" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" From 014ad4ce116876d294f904dffa1a6c3fc01ae2a7 Mon Sep 17 00:00:00 2001 From: Pooja V Date: Wed, 20 May 2026 15:39:12 +0530 Subject: [PATCH 3/3] address review comments --- .../services/realizestate/realize_state.go | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go index 4b2b4f755..952d4e20f 100644 --- a/pkg/nsx/services/realizestate/realize_state.go +++ b/pkg/nsx/services/realizestate/realize_state.go @@ -86,29 +86,6 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte }) } -func (service *RealizeStateService) GetPolicyInterfaceIP(realizedPath string) (string, error) { - result, err := service.NSXClient.RealizedEntityClient.Get(realizedPath) - err = nsxutil.TransNSXApiError(err) - if err != nil { - return "", err - } - - extendAttributes := result.ExtendedAttributes - for i := range extendAttributes { - if extendAttributes[i].Key != nil && *extendAttributes[i].Key == "IpAddresses" { - for _, ip := range extendAttributes[i].Values { - parts := strings.Split(ip, "/") - if len(parts) != 2 { - continue - } - return parts[0], nil - } - } - } - - return "", fmt.Errorf("%s LB SNAT IP not found", realizedPath) -} - // GetPolicyInterfaceIPs returns all bare IPs (with CIDR prefix stripped) from the // IpAddresses extended attribute on the realized entity. For dual-stack interfaces both // the IPv4 and IPv6 entries are returned.