diff --git a/pkg/controllers/backend/backend.go b/pkg/controllers/backend/backend.go index 7c29739d..139e0b83 100644 --- a/pkg/controllers/backend/backend.go +++ b/pkg/controllers/backend/backend.go @@ -146,41 +146,38 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI for _, ingress := range ingresses { for _, rule := range ingress.Spec.Rules { for _, path := range rule.HTTP.Paths { - svcName, svcPort, targetPort, _, err := util.PathToServiceAndTargetPort(c.serviceLister, ingress.Namespace, path) + pSvc, svc, err := util.ExtractServices(path, c.serviceLister, ingress) + if err != nil { + return err + } + svcName, svcPort, targetPort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, false) if err != nil { return err } - epAddrs, err := util.GetEndpoints(c.endpointLister, ingress.Namespace, svcName) if err != nil { return fmt.Errorf("unable to fetch endpoints for %s/%s/%d: %w", ingress.Namespace, svcName, targetPort, err) } - backends := []ociloadbalancer.BackendDetails{} for _, epAddr := range epAddrs { backends = append(backends, util.NewBackend(epAddr.IP, targetPort)) } - backendSetName := util.GenerateBackendSetName(ingress.Namespace, svcName, svcPort) err = c.client.GetLbClient().UpdateBackends(context.TODO(), lbID, backendSetName, backends) if err != nil { return fmt.Errorf("unable to update backends for %s/%s: %w", ingressClass.Name, backendSetName, err) } - backendSetHealth, err := c.client.GetLbClient().GetBackendSetHealth(context.TODO(), lbID, backendSetName) if err != nil { return fmt.Errorf("unable to fetch backendset health: %w", err) } - for _, epAddr := range epAddrs { pod, err := c.podLister.Pods(ingress.Namespace).Get(epAddr.TargetRef.Name) if err != nil { return fmt.Errorf("failed to fetch pod %s/%s: %w", ingress.Namespace, epAddr.TargetRef.Name, err) } - backendName := fmt.Sprintf("%s:%d", epAddr.IP, targetPort) readinessCondition := util.GetPodReadinessCondition(ingress.Name, rule.Host, path) - err = c.ensurePodReadinessCondition(pod, readinessCondition, backendSetHealth, backendName) if err != nil { return fmt.Errorf("%w", err) diff --git a/pkg/controllers/nodeBackend/nodeBackend.go b/pkg/controllers/nodeBackend/nodeBackend.go index 41b78ce9..8a54f7a0 100644 --- a/pkg/controllers/nodeBackend/nodeBackend.go +++ b/pkg/controllers/nodeBackend/nodeBackend.go @@ -169,17 +169,21 @@ func (c *Controller) ensureBackends(ingressClass *networkingv1.IngressClass, lbI for _, ingress := range ingresses { for _, rule := range ingress.Spec.Rules { for _, path := range rule.HTTP.Paths { - svcName, svcPort, _, svc, err := util.PathToServiceAndTargetPort(c.serviceLister, ingress.Namespace, path) + + pSvc, svc, err := util.ExtractServices(path, c.serviceLister, ingress) if err != nil { return err } - if svc == nil || svc.Spec.Ports == nil || svc.Spec.Ports[0].NodePort == 0 { + svcName, svcPort, nodePort, err := util.PathToServiceAndTargetPort(svc, pSvc, ingress.Namespace, true) + if err != nil { + return err + } + if svc == nil || svc.Spec.Ports == nil || nodePort == 0 { continue } var backends []ociloadbalancer.BackendDetails - nodePort := svc.Spec.Ports[0].NodePort trafficPolicy := svc.Spec.ExternalTrafficPolicy if trafficPolicy == corev1.ServiceExternalTrafficPolicyTypeCluster { for _, node := range nodes { diff --git a/pkg/util/util.go b/pkg/util/util.go index 6ffad8c9..2d9fca06 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -460,37 +460,56 @@ func PatchIngressClassWithAnnotation(client kubernetes.Interface, ic *networking return nil, false } -func GetTargetPortForService(lister corelisters.ServiceLister, namespace string, name string, port int32, portName string) (int32, int32, *corev1.Service, error) { - svc, err := lister.Services(namespace).Get(name) - if err != nil { - return 0, 0, nil, err - } - +func GetTargetPortForService(svc *corev1.Service, namespace string, name string, port int32, portName string, isNodeportRequired bool) (int32, int32, error) { for _, p := range svc.Spec.Ports { if (p.Port != 0 && p.Port == port) || p.Name == portName { - if p.TargetPort.Type != intstr.Int { - return 0, 0, nil, fmt.Errorf("service %s/%s has non-integer ports: %s", namespace, name, p.Name) + + if !isNodeportRequired { + if p.TargetPort.Type != intstr.Int { + return 0, 0, fmt.Errorf("service %s/%s has non-integer ports: %s", namespace, name, p.Name) + } + return p.Port, p.TargetPort.IntVal, nil + } else { + if p.NodePort == 0 { + return 0, 0, fmt.Errorf("service %s/%s has no nodeports: %s", namespace, name, p.Name) + } + return p.Port, p.NodePort, nil } - return p.Port, p.TargetPort.IntVal, svc, nil + } } - return 0, 0, nil, fmt.Errorf("service %s/%s does not have port: %s (%d)", namespace, name, portName, port) + return 0, 0, fmt.Errorf("service %s/%s does not have port: %s (%d)", namespace, name, portName, port) } -func PathToServiceAndTargetPort(lister corelisters.ServiceLister, ingressNamespace string, path networkingv1.HTTPIngressPath) (string, int32, int32, *corev1.Service, error) { - if path.Backend.Service == nil { - return "", 0, 0, nil, fmt.Errorf("backend service is not defined for ingress") +func PathToServiceAndTargetPort(svc *corev1.Service, svcBackend networkingv1.IngressServiceBackend, namespace string, isNodePortRequired bool) (string, int32, int32, error) { + + svcPort, targetPort, err := GetTargetPortForService(svc, namespace, svcBackend.Name, svcBackend.Port.Number, svcBackend.Port.Name, isNodePortRequired) + if err != nil { + return "", 0, 0, err } + return svcBackend.Name, svcPort, targetPort, nil +} - pSvc := *path.Backend.Service +func ExtractServices(path networkingv1.HTTPIngressPath, svcLister corelisters.ServiceLister, ingress *networkingv1.Ingress) (networkingv1.IngressServiceBackend, *corev1.Service, error) { + pSvc, err := getIngressBackend(path) + if err != nil { + return networkingv1.IngressServiceBackend{}, nil, err + } - svcPort, targetPort, svc, err := GetTargetPortForService(lister, ingressNamespace, pSvc.Name, pSvc.Port.Number, pSvc.Port.Name) + svc, err := svcLister.Services(ingress.Namespace).Get(pSvc.Name) if err != nil { - return "", 0, 0, nil, err + return networkingv1.IngressServiceBackend{}, nil, err } + return pSvc, svc, nil +} - return pSvc.Name, svcPort, targetPort, svc, nil +func getIngressBackend(path networkingv1.HTTPIngressPath) (networkingv1.IngressServiceBackend, error) { + if path.Backend.Service == nil { + return networkingv1.IngressServiceBackend{}, fmt.Errorf("backend service is not defined for ingress") + } + pSvc := *path.Backend.Service + return pSvc, nil } func GetEndpoints(lister corelisters.EndpointsLister, namespace string, service string) ([]corev1.EndpointAddress, error) { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 81082467..74aeff6b 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -11,6 +11,7 @@ package util import ( "context" "fmt" + "strconv" "strings" "testing" @@ -21,6 +22,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/informers" fakeclientset "k8s.io/client-go/kubernetes/fake" fake2 "k8s.io/client-go/kubernetes/typed/networking/v1/fake" @@ -562,6 +564,82 @@ func TestIsIngressDeleting(t *testing.T) { Expect(IsIngressDeleting(&i)).Should(Equal(false)) } +func TestPathToServiceAndTargetPort(t *testing.T) { + RegisterTestingT(t) + namespace := "test" + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "TestService", + Namespace: namespace, + }, + Spec: v1.ServiceSpec{ + Selector: map[string]string{"app": "multi-test"}, + Ports: []v1.ServicePort{ + { + Name: "abcd", + Protocol: v1.ProtocolTCP, + Port: 443, + TargetPort: intstr.IntOrString{ + IntVal: 8080, + }, + NodePort: 30224, + }, + { + Name: "efgh", + Protocol: v1.ProtocolTCP, + Port: 444, + TargetPort: intstr.IntOrString{ + IntVal: 8080, + }, + NodePort: 30225, + }, + { + Name: "ijkl", + Protocol: v1.ProtocolTCP, + Port: 445, + TargetPort: intstr.IntOrString{ + IntVal: 8080, + }, + NodePort: 30226, + }, + }, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal, + }, + } + ingBackend1 := networkingv1.IngressServiceBackend{ + Name: "TestService1", + Port: networkingv1.ServiceBackendPort{ + Number: 443, + Name: "abcd", + }, + } + ingBackend2 := networkingv1.IngressServiceBackend{ + Name: "TestService2", + Port: networkingv1.ServiceBackendPort{ + Number: 444, + Name: "efgh", + }, + } + ingBackend3 := networkingv1.IngressServiceBackend{ + Name: "TestService3", + Port: networkingv1.ServiceBackendPort{ + Number: 445, + }, + } + + runTests(svc, ingBackend1, namespace, "TestService1", "443", "30224") + runTests(svc, ingBackend2, namespace, "TestService2", "444", "30225") + runTests(svc, ingBackend3, namespace, "TestService3", "445", "30226") +} + +func runTests(svc *v1.Service, ingBackend networkingv1.IngressServiceBackend, namespace string, expectedSvcName string, expectedPort string, np string) { + svcName, svcPort, nodePort, err := PathToServiceAndTargetPort(svc, ingBackend, namespace, true) + Expect(err == nil).Should(Equal(true)) + Expect(svcName).Should(Equal(expectedSvcName)) + Expect(strconv.Itoa(int(svcPort))).Should(Equal(expectedPort)) + Expect(strconv.Itoa(int(nodePort))).Should(Equal(np)) +} + func getIngressClassList() *networkingv1.IngressClassList { defaultIngressClass := getIngressClassResource("default-ingress-class", true, "oci.oraclecloud.com/native-ingress-controller") ingressClass := getIngressClassResource("ingress-class", false, "oci.oraclecloud.com/native-ingress-controller")