Skip to content

Commit 1eded64

Browse files
Inbaraj-SInbaraj-S
authored
oke-30296-fix-multi-nodeport-bug (#52)
1 parent bb04d6f commit 1eded64

File tree

4 files changed

+126
-28
lines changed

4 files changed

+126
-28
lines changed

‎pkg/controllers/backend/backend.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,41 +146,38 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI
146146
for _, ingress := range ingresses {
147147
for _, rule := range ingress.Spec.Rules {
148148
for _, path := range rule.HTTP.Paths {
149-
svcName, svcPort, targetPort, _, err := util.PathToServiceAndTargetPort(c.serviceLister, ingress.Namespace, path)
149+
pSvc, svc, err := util.ExtractServices(path, c.serviceLister, ingress)
150+
if err != nil {
151+
return err
152+
}
153+
svcName, svcPort, targetPort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, false)
150154
if err != nil {
151155
return err
152156
}
153-
154157
epAddrs, err := util.GetEndpoints(c.endpointLister, ingress.Namespace, svcName)
155158
if err != nil {
156159
return fmt.Errorf("unable to fetch endpoints for %s/%s/%d: %w", ingress.Namespace, svcName, targetPort, err)
157160
}
158-
159161
backends := []ociloadbalancer.BackendDetails{}
160162
for _, epAddr := range epAddrs {
161163
backends = append(backends, util.NewBackend(epAddr.IP, targetPort))
162164
}
163-
164165
backendSetName := util.GenerateBackendSetName(ingress.Namespace, svcName, svcPort)
165166
err = c.client.GetLbClient().UpdateBackends(context.TODO(), lbID, backendSetName, backends)
166167
if err != nil {
167168
return fmt.Errorf("unable to update backends for %s/%s: %w", ingressClass.Name, backendSetName, err)
168169
}
169-
170170
backendSetHealth, err := c.client.GetLbClient().GetBackendSetHealth(context.TODO(), lbID, backendSetName)
171171
if err != nil {
172172
return fmt.Errorf("unable to fetch backendset health: %w", err)
173173
}
174-
175174
for _, epAddr := range epAddrs {
176175
pod, err := c.podLister.Pods(ingress.Namespace).Get(epAddr.TargetRef.Name)
177176
if err != nil {
178177
return fmt.Errorf("failed to fetch pod %s/%s: %w", ingress.Namespace, epAddr.TargetRef.Name, err)
179178
}
180-
181179
backendName := fmt.Sprintf("%s:%d", epAddr.IP, targetPort)
182180
readinessCondition := util.GetPodReadinessCondition(ingress.Name, rule.Host, path)
183-
184181
err = c.ensurePodReadinessCondition(pod, readinessCondition, backendSetHealth, backendName)
185182
if err != nil {
186183
return fmt.Errorf("%w", err)

‎pkg/controllers/nodeBackend/nodeBackend.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,17 +169,21 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI
169169
for _, ingress := range ingresses {
170170
for _, rule := range ingress.Spec.Rules {
171171
for _, path := range rule.HTTP.Paths {
172-
svcName, svcPort, _, svc, err := util.PathToServiceAndTargetPort(c.serviceLister, ingress.Namespace, path)
172+
173+
pSvc, svc, err := util.ExtractServices(path, c.serviceLister, ingress)
173174
if err != nil {
174175
return err
175176
}
176177

177-
if svc == nil || svc.Spec.Ports == nil || svc.Spec.Ports[0].NodePort == 0 {
178+
svcName, svcPort, nodePort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, true)
179+
if err != nil {
180+
return err
181+
}
182+
if svc == nil || svc.Spec.Ports == nil || nodePort == 0 {
178183
continue
179184
}
180185

181186
var backends []ociloadbalancer.BackendDetails
182-
nodePort := svc.Spec.Ports[0].NodePort
183187
trafficPolicy := svc.Spec.ExternalTrafficPolicy
184188
if trafficPolicy == corev1.ServiceExternalTrafficPolicyTypeCluster {
185189
for _, node := range nodes {

‎pkg/util/util.go

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -460,37 +460,56 @@ func PatchIngressClassWithAnnotation(client kubernetes.Interface, ic *networking
460460
return nil, false
461461
}
462462

463-
func GetTargetPortForService(lister corelisters.ServiceLister, namespace string, name string, port int32, portName string) (int32, int32, *corev1.Service, error) {
464-
svc, err := lister.Services(namespace).Get(name)
465-
if err != nil {
466-
return 0, 0, nil, err
467-
}
468-
463+
func GetTargetPortForService(svc *corev1.Service, namespace string, name string, port int32, portName string, isNodeportRequired bool) (int32, int32, error) {
469464
for _, p := range svc.Spec.Ports {
470465
if (p.Port != 0 && p.Port == port) || p.Name == portName {
471-
if p.TargetPort.Type != intstr.Int {
472-
return 0, 0, nil, fmt.Errorf("service %s/%s has non-integer ports: %s", namespace, name, p.Name)
466+
467+
if !isNodeportRequired {
468+
if p.TargetPort.Type != intstr.Int {
469+
return 0, 0, fmt.Errorf("service %s/%s has non-integer ports: %s", namespace, name, p.Name)
470+
}
471+
return p.Port, p.TargetPort.IntVal, nil
472+
} else {
473+
if p.NodePort == 0 {
474+
return 0, 0, fmt.Errorf("service %s/%s has no nodeports: %s", namespace, name, p.Name)
475+
}
476+
return p.Port, p.NodePort, nil
473477
}
474-
return p.Port, p.TargetPort.IntVal, svc, nil
478+
475479
}
476480
}
477481

478-
return 0, 0, nil, fmt.Errorf("service %s/%s does not have port: %s (%d)", namespace, name, portName, port)
482+
return 0, 0, fmt.Errorf("service %s/%s does not have port: %s (%d)", namespace, name, portName, port)
479483
}
480484

481-
func PathToServiceAndTargetPort(lister corelisters.ServiceLister, ingressNamespace string, path networkingv1.HTTPIngressPath) (string, int32, int32, *corev1.Service, error) {
482-
if path.Backend.Service == nil {
483-
return "", 0, 0, nil, fmt.Errorf("backend service is not defined for ingress")
485+
func PathToServiceAndTargetPort(svc *corev1.Service, svcBackend networkingv1.IngressServiceBackend, namespace string, isNodePortRequired bool) (string, int32, int32, error) {
486+
487+
svcPort, targetPort, err := GetTargetPortForService(svc, namespace, svcBackend.Name, svcBackend.Port.Number, svcBackend.Port.Name, isNodePortRequired)
488+
if err != nil {
489+
return "", 0, 0, err
484490
}
491+
return svcBackend.Name, svcPort, targetPort, nil
492+
}
485493

486-
pSvc := *path.Backend.Service
494+
func ExtractServices(path networkingv1.HTTPIngressPath, svcLister corelisters.ServiceLister, ingress *networkingv1.Ingress) (networkingv1.IngressServiceBackend, *corev1.Service, error) {
495+
pSvc, err := getIngressBackend(path)
496+
if err != nil {
497+
return networkingv1.IngressServiceBackend{}, nil, err
498+
}
487499

488-
svcPort, targetPort, svc, err := GetTargetPortForService(lister, ingressNamespace, pSvc.Name, pSvc.Port.Number, pSvc.Port.Name)
500+
svc, err := svcLister.Services(ingress.Namespace).Get(pSvc.Name)
489501
if err != nil {
490-
return "", 0, 0, nil, err
502+
return networkingv1.IngressServiceBackend{}, nil, err
491503
}
504+
return pSvc, svc, nil
505+
}
492506

493-
return pSvc.Name, svcPort, targetPort, svc, nil
507+
func getIngressBackend(path networkingv1.HTTPIngressPath) (networkingv1.IngressServiceBackend, error) {
508+
if path.Backend.Service == nil {
509+
return networkingv1.IngressServiceBackend{}, fmt.Errorf("backend service is not defined for ingress")
510+
}
511+
pSvc := *path.Backend.Service
512+
return pSvc, nil
494513
}
495514

496515
func GetEndpoints(lister corelisters.EndpointsLister, namespace string, service string) ([]corev1.EndpointAddress, error) {

‎pkg/util/util_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package util
1111
import (
1212
"context"
1313
"fmt"
14+
"strconv"
1415
"strings"
1516
"testing"
1617

@@ -21,6 +22,7 @@ import (
2122
networkingv1 "k8s.io/api/networking/v1"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/runtime"
25+
"k8s.io/apimachinery/pkg/util/intstr"
2426
"k8s.io/client-go/informers"
2527
fakeclientset "k8s.io/client-go/kubernetes/fake"
2628
fake2 "k8s.io/client-go/kubernetes/typed/networking/v1/fake"
@@ -562,6 +564,82 @@ func TestIsIngressDeleting(t *testing.T) {
562564
Expect(IsIngressDeleting(&i)).Should(Equal(false))
563565
}
564566

567+
func TestPathToServiceAndTargetPort(t *testing.T) {
568+
RegisterTestingT(t)
569+
namespace := "test"
570+
svc := &v1.Service{
571+
ObjectMeta: metav1.ObjectMeta{
572+
Name: "TestService",
573+
Namespace: namespace,
574+
},
575+
Spec: v1.ServiceSpec{
576+
Selector: map[string]string{"app": "multi-test"},
577+
Ports: []v1.ServicePort{
578+
{
579+
Name: "abcd",
580+
Protocol: v1.ProtocolTCP,
581+
Port: 443,
582+
TargetPort: intstr.IntOrString{
583+
IntVal: 8080,
584+
},
585+
NodePort: 30224,
586+
},
587+
{
588+
Name: "efgh",
589+
Protocol: v1.ProtocolTCP,
590+
Port: 444,
591+
TargetPort: intstr.IntOrString{
592+
IntVal: 8080,
593+
},
594+
NodePort: 30225,
595+
},
596+
{
597+
Name: "ijkl",
598+
Protocol: v1.ProtocolTCP,
599+
Port: 445,
600+
TargetPort: intstr.IntOrString{
601+
IntVal: 8080,
602+
},
603+
NodePort: 30226,
604+
},
605+
},
606+
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
607+
},
608+
}
609+
ingBackend1 := networkingv1.IngressServiceBackend{
610+
Name: "TestService1",
611+
Port: networkingv1.ServiceBackendPort{
612+
Number: 443,
613+
Name: "abcd",
614+
},
615+
}
616+
ingBackend2 := networkingv1.IngressServiceBackend{
617+
Name: "TestService2",
618+
Port: networkingv1.ServiceBackendPort{
619+
Number: 444,
620+
Name: "efgh",
621+
},
622+
}
623+
ingBackend3 := networkingv1.IngressServiceBackend{
624+
Name: "TestService3",
625+
Port: networkingv1.ServiceBackendPort{
626+
Number: 445,
627+
},
628+
}
629+
630+
runTests(svc, ingBackend1, namespace, "TestService1", "443", "30224")
631+
runTests(svc, ingBackend2, namespace, "TestService2", "444", "30225")
632+
runTests(svc, ingBackend3, namespace, "TestService3", "445", "30226")
633+
}
634+
635+
func runTests(svc *v1.Service, ingBackend networkingv1.IngressServiceBackend, namespace string, expectedSvcName string, expectedPort string, np string) {
636+
svcName, svcPort, nodePort, err := PathToServiceAndTargetPort(svc, ingBackend, namespace, true)
637+
Expect(err == nil).Should(Equal(true))
638+
Expect(svcName).Should(Equal(expectedSvcName))
639+
Expect(strconv.Itoa(int(svcPort))).Should(Equal(expectedPort))
640+
Expect(strconv.Itoa(int(nodePort))).Should(Equal(np))
641+
}
642+
565643
func getIngressClassList() *networkingv1.IngressClassList {
566644
defaultIngressClass := getIngressClassResource("default-ingress-class", true, "oci.oraclecloud.com/native-ingress-controller")
567645
ingressClass := getIngressClassResource("ingress-class", false, "oci.oraclecloud.com/native-ingress-controller")

0 commit comments

Comments
 (0)