From d4e49b2eed7a30320f4acccd82be050e3a6640bd Mon Sep 17 00:00:00 2001 From: Prince Pereira Date: Tue, 1 Jul 2025 02:25:37 -0700 Subject: [PATCH] Ensure Loadbalancer internal port is set to container port from endpointslice if the targetPort is not specified in service info creation. --- pkg/proxy/winkernel/proxier.go | 4 +- pkg/proxy/winkernel/proxier_test.go | 69 +++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 91fb6fad8d4..df09ec78faf 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -1356,7 +1356,9 @@ func (proxier *Proxier) syncProxyRules() { // targetPort is zero if it is specified as a name in port.TargetPort, so the real port should be got from endpoints. // Note that hnslib.AddLoadBalancer() doesn't support endpoints with different ports, so only port from first endpoint is used. // TODO(feiskyer): add support of different endpoint ports after hnslib.AddLoadBalancer() add that. - if svcInfo.targetPort == 0 { + if svcInfo.targetPort == 0 || svcInfo.targetPort != int(ep.port) { + // Update the targetPort to the first endpoint's port if it is not specified or different from the endpoint's port. + klog.V(3).InfoS("Update targetPort", "oldTargetPort", svcInfo.targetPort, "newTargetPort", ep.port) svcInfo.targetPort = int(ep.port) } // There is a bug in Windows Server 2019 that can cause two endpoints to be created with the same IP address, so we need to check using endpoint ID first. diff --git a/pkg/proxy/winkernel/proxier_test.go b/pkg/proxy/winkernel/proxier_test.go index f84526a412a..50070fba369 100644 --- a/pkg/proxy/winkernel/proxier_test.go +++ b/pkg/proxy/winkernel/proxier_test.go @@ -27,6 +27,7 @@ import ( "testing" "github.com/Microsoft/hnslib/hcn" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" @@ -1306,6 +1307,74 @@ func TestClusterIPLBInCreateDsrLoadBalancer(t *testing.T) { } } +func TestEndpointSliceWithInternalPortDifferentFromServicePort(t *testing.T) { + proxier := NewFakeProxier(t, testNodeName, netutils.ParseIPSloppy("10.0.0.1"), NETWORK_TYPE_OVERLAY, true) + assert.NotNil(t, proxier, "Failed to create proxier") + + proxier.servicesSynced = true + proxier.endpointSlicesSynced = true + + svcPortName := proxy.ServicePortName{ + NamespacedName: makeNSN("ns1", "svc1"), + Port: "p80", + Protocol: v1.ProtocolTCP, + } + + svcSpec := v1.ServiceSpec{ + ClusterIP: "172.20.1.1", + Selector: map[string]string{"foo": "bar"}, + Ports: []v1.ServicePort{ + {Name: svcPortName.Port, Port: 80, TargetPort: intstr.FromInt32(80), Protocol: v1.ProtocolTCP}, // Mocking TargetPort as to same as service port (80) + }, + } + + proxier.OnServiceAdd(&v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: svcPortName.Name, Namespace: svcPortName.Namespace}, + Spec: svcSpec, + }) + + // Add initial endpoint slice + endpointSlice := &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-1", svcPortName.Name), + Namespace: svcPortName.Namespace, + Labels: map[string]string{discovery.LabelServiceName: svcPortName.Name}, + }, + Ports: []discovery.EndpointPort{{ + Name: &svcPortName.Port, + Port: ptr.To[int32](8080), // Using container port 8080 which is different from service port 80 + Protocol: ptr.To(v1.ProtocolTCP), + }}, + AddressType: discovery.AddressTypeIPv4, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"192.168.2.3"}, + Conditions: discovery.EndpointConditions{Ready: ptr.To(true)}, + NodeName: ptr.To("testhost2"), + }}, + } + + proxier.OnEndpointSliceAdd(endpointSlice) + proxier.setInitialized(true) + proxier.syncProxyRules() + + svc := proxier.svcPortMap[svcPortName] + svcInfo, ok := svc.(*serviceInfo) + assert.True(t, ok, "Failed to cast serviceInfo %q", svcPortName.String()) + assert.Equal(t, svcInfo.hnsID, loadbalancerGuid1, "The Hns Loadbalancer Id %v does not match %v. ServicePortName %q", svcInfo.hnsID, loadbalancerGuid1, svcPortName.String()) + + lb, err := proxier.hcn.GetLoadBalancerByID(svcInfo.hnsID) + assert.Equal(t, nil, err, "Failed to fetch loadbalancer: %v", err) + assert.NotEqual(t, nil, lb, "Loadbalancer object should not be nil") + assert.Equal(t, len(lb.PortMappings), 1, "PortMappings should have one and only one entry") + assert.Equal(t, lb.PortMappings[0].InternalPort, uint16(8080), "InternalPort should be 8080") + assert.Equal(t, lb.PortMappings[0].ExternalPort, uint16(80), "ExternalPort should be 80") + + ep := proxier.endpointsMap[svcPortName][0] + epInfo, ok := ep.(*endpointInfo) + assert.True(t, ok, "Failed to cast endpointInfo %q", svcPortName.String()) + assert.Equal(t, epInfo.hnsID, "EPID-3", "Hns EndpointId %v does not match %v. ServicePortName %q", epInfo.hnsID, endpointGuid1, svcPortName.String()) +} + func TestEndpointSlice(t *testing.T) { proxier := NewFakeProxier(t, testNodeName, netutils.ParseIPSloppy("10.0.0.1"), NETWORK_TYPE_OVERLAY, true) if proxier == nil {