Skip to content

Commit c54cf0a

Browse files
piyush-tiwaripiyush-tiwari
committed
fix targetPort fetch logic for NPN backends
- Fixed targetPort fetch logic for NPN backends when targetPort for a service is specified as its name - Similar fix for defaultBackend logic as well - Added Unit tests in backend_test.go and util/util_test.go
1 parent 21b3de4 commit c54cf0a

File tree

9 files changed

+358
-80
lines changed

9 files changed

+358
-80
lines changed

‎pkg/controllers/backend/backend.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI
149149
if err != nil {
150150
return err
151151
}
152-
svcName, svcPort, targetPort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, false)
152+
svcName, svcPort, targetPort, err := util.PathToServiceAndTargetPort(c.endpointLister, svc, pSvc, ingress.Namespace, false)
153153
if err != nil {
154154
return err
155155
}
@@ -235,8 +235,16 @@ func (c *Controller) getDefaultBackends(ingresses []*networkingv1.Ingress) ([]oc
235235

236236
}
237237

238-
svcName := backend.Service.Name
239-
targetPort := backend.Service.Port.Number
238+
svc, err := c.serviceLister.Services(namespace).Get(backend.Service.Name)
239+
if err != nil {
240+
return nil, err
241+
}
242+
243+
svcName, _, targetPort, err := util.PathToServiceAndTargetPort(c.endpointLister, svc, *backend.Service, namespace, false)
244+
if err != nil {
245+
return nil, err
246+
}
247+
240248
epAdrress, err := util.GetEndpoints(c.endpointLister, namespace, svcName)
241249
if err != nil {
242250
return nil, fmt.Errorf("unable to fetch endpoints for %s/%s/%d: %w", namespace, svcName, targetPort, err)

‎pkg/controllers/backend/backendPathWithNamedTargetPortDefaultBackend.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
apiVersion: networking.k8s.io/v1
3+
kind: Ingress
4+
metadata:
5+
name: ingress-readiness
6+
namespace: default
7+
spec:
8+
defaultBackend:
9+
service:
10+
name: service-with-named-target-port
11+
port:
12+
number: 8081
13+
rules:
14+
- host: "foo.bar.com"
15+
http:
16+
paths:
17+
- pathType: Exact
18+
path: "/testecho1"
19+
backend:
20+
service:
21+
name: testecho1
22+
port:
23+
number: 80

‎pkg/controllers/backend/backendPathWithNamedTargetPortService.yaml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
apiVersion: networking.k8s.io/v1
3+
kind: Ingress
4+
metadata:
5+
name: ingress-readiness
6+
namespace: default
7+
spec:
8+
rules:
9+
- host: "foo.bar.com"
10+
http:
11+
paths:
12+
- pathType: Exact
13+
path: "/service-with-named-target-port"
14+
backend:
15+
service:
16+
name: service-with-named-target-port
17+
port:
18+
number: 8081
19+
- host: "foo.bar.com1"
20+
http:
21+
paths:
22+
- pathType: Exact
23+
path: "/service-with-numeric-target-port"
24+
backend:
25+
service:
26+
name: service-with-named-target-port
27+
port:
28+
name: "port_8082"

‎pkg/controllers/backend/backend_test.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ import (
2626
)
2727

2828
const (
29-
backendPath = "backendPath.yaml"
30-
backendPathWithDefaultBackend = "backendPathWithDefaultBackend.yaml"
31-
namespace = "default"
29+
backendPath = "backendPath.yaml"
30+
backendPathWithDefaultBackend = "backendPathWithDefaultBackend.yaml"
31+
backendPathWithNamedTargetPortService = "backendPathWithNamedTargetPortService.yaml"
32+
backendPathWithNamedTargetPortDefaultBackend = "backendPathWithNamedTargetPortDefaultBackend.yaml"
33+
namespace = "default"
3234
)
3335

3436
func setUp(ctx context.Context, ingressClassList *networkingv1.IngressClassList, ingressList *networkingv1.IngressList, testService *corev1.ServiceList, endpoints *corev1.EndpointsList, pod *corev1.PodList) (networkinginformers.IngressClassInformer, networkinginformers.IngressInformer, corelisters.ServiceLister, corelisters.EndpointsLister, corelisters.PodLister, *fakeclientset.Clientset) {
@@ -126,6 +128,7 @@ func TestNoDefaultBackends(t *testing.T) {
126128
Expect(err == nil).Should(Equal(true))
127129
Expect(len(backends)).Should(Equal(0))
128130
}
131+
129132
func TestDefaultBackends(t *testing.T) {
130133
RegisterTestingT(t)
131134
ctx, cancel := context.WithCancel(context.Background())
@@ -138,6 +141,28 @@ func TestDefaultBackends(t *testing.T) {
138141
Expect(len(backends)).Should(Equal(1))
139142
}
140143

144+
func TestDefaultBackendsWithNamedTargetPort(t *testing.T) {
145+
RegisterTestingT(t)
146+
ctx, cancel := context.WithCancel(context.Background())
147+
defer cancel()
148+
ingressClassList := util.GetIngressClassList()
149+
c := inits(ctx, ingressClassList, backendPathWithNamedTargetPortDefaultBackend)
150+
ingresses, _ := util.GetIngressesForClass(c.ingressLister, &ingressClassList.Items[0])
151+
backends, err := c.getDefaultBackends(ingresses)
152+
Expect(err == nil).Should(Equal(true))
153+
Expect(len(backends)).Should(Equal(1))
154+
}
155+
156+
func TestEnsureBackendWithNamedTargetPort(t *testing.T) {
157+
RegisterTestingT(t)
158+
ctx, cancel := context.WithCancel(context.Background())
159+
defer cancel()
160+
ingressClassList := util.GetIngressClassList()
161+
c := inits(ctx, ingressClassList, backendPathWithNamedTargetPortService)
162+
err := c.ensureBackends(&ingressClassList.Items[0], "id")
163+
Expect(err == nil).Should(Equal(true))
164+
}
165+
141166
func TestEnsurePodReadinessConditionWithExistingReadiness(t *testing.T) {
142167
RegisterTestingT(t)
143168
ctx, cancel := context.WithCancel(context.Background())

‎pkg/controllers/ingress/ingress_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,10 @@ func (m MockLoadBalancerClient) UpdateBackendSet(ctx context.Context, request oc
258258
}
259259

260260
func (m MockLoadBalancerClient) DeleteBackendSet(ctx context.Context, request ociloadbalancer.DeleteBackendSetRequest) (ociloadbalancer.DeleteBackendSetResponse, error) {
261-
return ociloadbalancer.DeleteBackendSetResponse{}, nil
261+
return ociloadbalancer.DeleteBackendSetResponse{
262+
OpcRequestId: common.String("OpcRequestId"),
263+
OpcWorkRequestId: common.String("OpcWorkRequestId"),
264+
}, nil
262265
}
263266

264267
func (m MockLoadBalancerClient) GetBackendSetHealth(ctx context.Context, request ociloadbalancer.GetBackendSetHealthRequest) (ociloadbalancer.GetBackendSetHealthResponse, error) {

‎pkg/controllers/nodeBackend/nodeBackend.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI
173173
return err
174174
}
175175

176-
svcName, svcPort, nodePort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, true)
176+
svcName, svcPort, nodePort, err := util.PathToServiceAndTargetPort(c.endpointLister, svc, pSvc, ingress.Namespace, true)
177177
if err != nil {
178178
return err
179179
}

‎pkg/util/testutil.go‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package util
22

33
import (
4+
"context"
45
"encoding/base64"
56
"fmt"
7+
"k8s.io/apimachinery/pkg/util/intstr"
8+
"k8s.io/client-go/informers"
9+
corelisters "k8s.io/client-go/listers/core/v1"
10+
"k8s.io/client-go/tools/cache"
611
"log"
712
"os"
813
"strings"
@@ -87,14 +92,42 @@ func GetServiceListResource(namespace string, name string, port int32) *v1.Servi
8792
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
8893
},
8994
}
95+
testService3 := v1.Service{
96+
ObjectMeta: metav1.ObjectMeta{
97+
Name: "service-with-named-target-port",
98+
Namespace: namespace,
99+
},
100+
Spec: v1.ServiceSpec{
101+
Selector: map[string]string{"app": name},
102+
Ports: []v1.ServicePort{
103+
{
104+
Protocol: v1.ProtocolTCP,
105+
Port: 8081,
106+
NodePort: 30225,
107+
TargetPort: intstr.FromString("test_port_name"),
108+
Name: "port_8081",
109+
},
110+
{
111+
Protocol: v1.ProtocolTCP,
112+
Port: 8082,
113+
NodePort: 30226,
114+
TargetPort: intstr.FromInt(1001),
115+
Name: "port_8082",
116+
},
117+
},
118+
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
119+
},
120+
}
90121
var services []v1.Service
91122
services = append(services, testService)
92123
services = append(services, testService2)
124+
services = append(services, testService3)
93125

94126
return &v1.ServiceList{
95127
Items: services,
96128
}
97129
}
130+
98131
func GetServiceListResourceWithPortName(namespace string, name string, port int32, portName string) *v1.ServiceList {
99132
testService := v1.Service{
100133
ObjectMeta: metav1.ObjectMeta{
@@ -239,10 +272,32 @@ func GetEndpointsResourceList(name string, namespace string, allCase bool) *v1.E
239272
Ports: []v1.EndpointPort{{Port: 1000}},
240273
}},
241274
}
275+
endpoint3 := v1.Endpoints{
276+
ObjectMeta: metav1.ObjectMeta{
277+
Name: "service-with-named-target-port",
278+
Namespace: namespace,
279+
ResourceVersion: "1",
280+
},
281+
Subsets: []v1.EndpointSubset{{
282+
Addresses: []v1.EndpointAddress{{
283+
IP: "6.7.8.9",
284+
Hostname: "",
285+
NodeName: &emptyNodeName,
286+
TargetRef: &v1.ObjectReference{
287+
Kind: "Pod",
288+
Namespace: "default",
289+
Name: "testpod",
290+
UID: "999",
291+
},
292+
}},
293+
Ports: []v1.EndpointPort{{Port: 1000, Name: "port_8081"}, {Port: 1001, Name: "port_8082"}},
294+
}},
295+
}
242296

243297
var endpoints []v1.Endpoints
244298
endpoints = append(endpoints, endpoint)
245299
endpoints = append(endpoints, endpoint2)
300+
endpoints = append(endpoints, endpoint3)
246301

247302
return &v1.EndpointsList{
248303
Items: endpoints,
@@ -326,11 +381,33 @@ func GetEndpointsResourceListAllCase(name string, namespace string) *v1.Endpoint
326381
}},
327382
}},
328383
}
384+
endpoint4 := v1.Endpoints{
385+
ObjectMeta: metav1.ObjectMeta{
386+
Name: "service-with-named-target-port",
387+
Namespace: namespace,
388+
ResourceVersion: "1",
389+
},
390+
Subsets: []v1.EndpointSubset{{
391+
Addresses: []v1.EndpointAddress{{
392+
IP: "6.7.8.9",
393+
Hostname: "",
394+
NodeName: &emptyNodeName,
395+
TargetRef: &v1.ObjectReference{
396+
Kind: "Pod",
397+
Namespace: "default",
398+
Name: "testpod",
399+
UID: "999",
400+
},
401+
}},
402+
Ports: []v1.EndpointPort{{Port: 1000, Name: "port_8081"}, {Port: 1001, Name: "port_8082"}},
403+
}},
404+
}
329405

330406
var endpoints []v1.Endpoints
331407
endpoints = append(endpoints, endpoint)
332408
endpoints = append(endpoints, endpoint2)
333409
endpoints = append(endpoints, endpoint3)
410+
endpoints = append(endpoints, endpoint4)
334411

335412
return &v1.EndpointsList{
336413
Items: endpoints,
@@ -432,6 +509,8 @@ func SampleLoadBalancerResponse() ociloadbalancer.GetLoadBalancerResponse {
432509
etag := "testTag"
433510
lbId := "id"
434511
backendSetName := GenerateBackendSetName("default", "testecho1", 80)
512+
backendSetName2 := GenerateBackendSetName("default", "service-with-named-target-port", 8081)
513+
backendSetName3 := GenerateBackendSetName("default", "service-with-named-target-port", 8082)
435514
name := "testecho1-999"
436515
port := 80
437516
ip := "127.89.90.90"
@@ -519,6 +598,24 @@ func SampleLoadBalancerResponse() ociloadbalancer.GetLoadBalancerResponse {
519598
SessionPersistenceConfiguration: nil,
520599
LbCookieSessionPersistenceConfiguration: nil,
521600
},
601+
backendSetName2: {
602+
Name: &backendSetName2,
603+
Policy: &policy,
604+
Backends: backends,
605+
HealthChecker: healthChecker,
606+
SslConfiguration: nil,
607+
SessionPersistenceConfiguration: nil,
608+
LbCookieSessionPersistenceConfiguration: nil,
609+
},
610+
backendSetName3: {
611+
Name: &backendSetName3,
612+
Policy: &policy,
613+
Backends: backends,
614+
HealthChecker: healthChecker,
615+
SslConfiguration: nil,
616+
SessionPersistenceConfiguration: nil,
617+
LbCookieSessionPersistenceConfiguration: nil,
618+
},
522619
},
523620
PathRouteSets: nil,
524621
FreeformTags: nil,
@@ -673,3 +770,66 @@ func GetNodesList() *v1.NodeList {
673770
Items: nodes,
674771
}
675772
}
773+
774+
func GetServicePortResource(name string, port int32, targetPort intstr.IntOrString, nodePort int32) v1.ServicePort {
775+
return v1.ServicePort{
776+
Name: name,
777+
Port: port,
778+
TargetPort: targetPort,
779+
NodePort: nodePort,
780+
}
781+
}
782+
783+
func GetServiceResource(namespace string, name string, ports []v1.ServicePort) *v1.Service {
784+
return &v1.Service{
785+
ObjectMeta: metav1.ObjectMeta{
786+
Name: name,
787+
Namespace: namespace,
788+
},
789+
Spec: v1.ServiceSpec{
790+
Selector: map[string]string{"app": name},
791+
Ports: ports,
792+
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
793+
},
794+
}
795+
}
796+
797+
func GetEndpointsResource(namespace string, name string, ports []v1.EndpointPort) v1.Endpoints {
798+
var emptyNodeName string
799+
return v1.Endpoints{
800+
ObjectMeta: metav1.ObjectMeta{
801+
Name: name,
802+
Namespace: namespace,
803+
ResourceVersion: "1",
804+
},
805+
Subsets: []v1.EndpointSubset{{
806+
Addresses: []v1.EndpointAddress{{IP: "6.7.8.9", NodeName: &emptyNodeName}},
807+
Ports: ports,
808+
}},
809+
}
810+
}
811+
812+
func GetIngressServiceBackendResource(name string, portName string, portNumber int32) networkingv1.IngressServiceBackend {
813+
return networkingv1.IngressServiceBackend{
814+
Name: name,
815+
Port: networkingv1.ServiceBackendPort{
816+
Name: portName,
817+
Number: portNumber,
818+
},
819+
}
820+
}
821+
822+
func GetEndpointsListerResource(endpointsList *v1.EndpointsList) corelisters.EndpointsLister {
823+
ctx, cancel := context.WithCancel(context.Background())
824+
defer cancel()
825+
826+
client := fakeclientset.NewSimpleClientset()
827+
UpdateFakeClientCall(client, "list", "endpoints", endpointsList)
828+
informerFactory := informers.NewSharedInformerFactory(client, 0)
829+
endpointInformer := informerFactory.Core().V1().Endpoints()
830+
endpointsLister := endpointInformer.Lister()
831+
informerFactory.Start(ctx.Done())
832+
cache.WaitForCacheSync(ctx.Done(), endpointInformer.Informer().HasSynced)
833+
834+
return endpointsLister
835+
}

0 commit comments

Comments
 (0)