diff --git a/pkg/controllers/networkinfo/networkinfo_controller.go b/pkg/controllers/networkinfo/networkinfo_controller.go index 76c0f5342..167b7ae3f 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,31 @@ 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 +// valid IPv6 entry, and return "" when no valid IP is present. +func primaryLBIP(ips []string) string { + var firstValid string + for _, ip := range ips { + raw := ip + if idx := strings.Index(ip, "/"); idx != -1 { + raw = ip[:idx] + } + parsed := net.ParseIP(raw) + if parsed == nil { + continue + } + if parsed.To4() != nil { + return ip + } + if firstValid == "" { + firstValid = ip + } + } + return firstValid } 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..8b46c4b22 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,8 +1111,8 @@ 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,9 +1183,9 @@ 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) { + 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 + return nil, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkStackFromNC", func(_ *vpc.VPCService, _ *v1alpha1.VPCNetworkConfiguration) (v1alpha1.NetworkStackType, error) { return v1alpha1.FullStackVPC, nil @@ -1275,8 +1275,8 @@ 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,72 @@ 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", + }, + { + name: "invalid IP before valid IPv6 is skipped in fallback", + ips: []string{"not-an-ip", "2001:db8::1"}, + want: "2001:db8::1", + }, + { + name: "all invalid IPs returns empty string", + ips: []string{"not-an-ip", "also-invalid"}, + want: "", + }, + } + 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..952d4e20f 100644 --- a/pkg/nsx/services/realizestate/realize_state.go +++ b/pkg/nsx/services/realizestate/realize_state.go @@ -86,25 +86,31 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte }) } -func (service *RealizeStateService) GetPolicyInterfaceIP(realizedPath string) (string, error) { +// 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 "", err + return nil, err } - extendAttributes := result.ExtendedAttributes - for i := range extendAttributes { - if extendAttributes[i].Key != nil && *extendAttributes[i].Key == "IpAddresses" { - for _, ip := range extendAttributes[i].Values { + 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 } - return parts[0], nil + ips = append(ips, parts[0]) } } } - return "", fmt.Errorf("%s LB SNAT IP not found", realizedPath) + 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..29dbd4d70 100644 --- a/pkg/nsx/services/vpc/vpc.go +++ b/pkg/nsx/services/vpc/vpc.go @@ -32,6 +32,7 @@ const ( AVILB = LBProvider("avi") NoneLB = LBProvider("none") 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..952eb03ca 100644 --- a/pkg/nsx/services/vpc/vpc_test.go +++ b/pkg/nsx/services/vpc/vpc_test.go @@ -12,6 +12,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + 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" @@ -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") +}