diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 81ea933..f172c47 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -37,7 +37,7 @@ type Storer interface { ListIngress() []*networkv1.Ingress GetService(key string) (*corev1.Service, error) GetNodesIpList() []string - GetIngressServiceInfo(ingress *networkv1.Ingress) (map[string]ServiceInfo, error) + GetIngressHostsInfo(ingress *networkv1.Ingress) (map[string]HostInfo, error) } // Store represents cache store, implements Storer @@ -75,8 +75,8 @@ func (s *Store) GetNodesIpList() []string { } // GetIngressServiceInfo returns ingress services info. -func (s *Store) GetIngressServiceInfo(ingress *networkv1.Ingress) (map[string]ServiceInfo, error) { - return getIngressServiceInfo(ingress, s) +func (s *Store) GetIngressHostsInfo(ingress *networkv1.Ingress) (map[string]HostInfo, error) { + return getIngressHostsInfo(ingress, s) } type Informer struct { diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 5e97221..6f9ec11 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -1,7 +1,7 @@ package store import ( - "errors" + "fmt" "testing" "time" @@ -144,6 +144,7 @@ func TestGetIngressServiceInfo(t *testing.T) { } s.listers.Node.Add(node1) + hostname := "example.com" serviceName := "test-service" service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -172,7 +173,7 @@ func TestGetIngressServiceInfo(t *testing.T) { Spec: networkv1.IngressSpec{ Rules: []networkv1.IngressRule{ { - Host: "example.com", + Host: hostname, IngressRuleValue: networkv1.IngressRuleValue{ HTTP: &networkv1.HTTPIngressRuleValue{ Paths: []networkv1.HTTPIngressPath{ @@ -193,20 +194,22 @@ func TestGetIngressServiceInfo(t *testing.T) { }, } - serviceInfo, err := s.GetIngressServiceInfo(ingress) + hostsInfo, err := s.GetIngressHostsInfo(ingress) g.Expect(err).To(BeNil()) + g.Expect(hostsInfo[hostname].Paths).ToNot(BeEmpty()) - g.Expect(serviceInfo).To(HaveKey(serviceName)) - g.Expect(serviceInfo[serviceName].Hosts).To(ContainElement("example.com")) - g.Expect(serviceInfo[serviceName].NodePort).To(Equal(30000)) - g.Expect(serviceInfo[serviceName].NodeIps).To(ConsistOf("192.168.1.1")) - g.Expect(serviceInfo[serviceName].Annotations).To(HaveKeyWithValue("key", "value")) + p := hostsInfo[hostname].Paths[0] + + g.Expect(p.Service.Name).To(Equal(serviceName)) + g.Expect(p.NodePort).To(Equal(30000)) + g.Expect(p.NodeIps).To(ConsistOf("192.168.1.1")) + g.Expect(p.Service.Annotations).To(HaveKeyWithValue("key", "value")) // check if service doens't have NodePort service.Spec.Ports[0].NodePort = 0 s.listers.Service.Update(service) - serviceInfo, err = s.GetIngressServiceInfo(ingress) - expectedErr := errors.New("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported") + _, err = s.GetIngressHostsInfo(ingress) + expectedErr := fmt.Errorf("service %s has no NodePort (only NodePort/LoadBalancer supported)", serviceName) g.Expect(err).To(Equal(expectedErr)) } diff --git a/internal/ingress/controller/store/utils.go b/internal/ingress/controller/store/utils.go index cf93e9b..fa2baee 100644 --- a/internal/ingress/controller/store/utils.go +++ b/internal/ingress/controller/store/utils.go @@ -3,20 +3,27 @@ package store import ( "fmt" + corev1 "k8s.io/api/core/v1" networkv1 "k8s.io/api/networking/v1" ) -// ServiceInfo represents helper struct for ingress service -type ServiceInfo struct { - Hosts []string - NodeIps []string - NodePort int - Annotations map[string]string +// PathInfo represents info about a path in the ingress controller +type PathInfo struct { + Path string + Service *corev1.Service + NodePort int + NodeIps []string } -// GetIngressServiceInfo get services info from ingress -func getIngressServiceInfo(ingress *networkv1.Ingress, store Storer) (map[string]ServiceInfo, error) { - servicesInfo := make(map[string]ServiceInfo) +// HostInfo represents info about a host in the ingress controller +type HostInfo struct { + Host string + Paths []PathInfo +} + +// getIngressHostsInfo get hosts info from ingress +func getIngressHostsInfo(ingress *networkv1.Ingress, store Storer) (map[string]HostInfo, error) { + hostsInfo := make(map[string]HostInfo) nodeIps := store.GetNodesIpList() for _, rule := range ingress.Spec.Rules { @@ -24,36 +31,41 @@ func getIngressServiceInfo(ingress *networkv1.Ingress, store Storer) (map[string continue } + hInfo := hostsInfo[rule.Host] + hInfo.Host = rule.Host + for _, path := range rule.HTTP.Paths { - service, err := store.GetService(ingress.Namespace + "/" + path.Backend.Service.Name) + svc, err := store.GetService(ingress.Namespace + "/" + path.Backend.Service.Name) if err != nil { return nil, fmt.Errorf("error getting service: %v", err) } - for _, port := range service.Spec.Ports { + var nodePort int32 + found := false + for _, port := range svc.Spec.Ports { if port.Port == path.Backend.Service.Port.Number { - if port.NodePort != 0 { - serviceName := path.Backend.Service.Name - if _, ok := servicesInfo[serviceName]; !ok { - servicesInfo[serviceName] = ServiceInfo{ - Hosts: []string{rule.Host}, - NodePort: int(port.NodePort), - NodeIps: nodeIps, - Annotations: service.Annotations, - } - } else { - sTmp := servicesInfo[serviceName] - sTmp.Hosts = append(sTmp.Hosts, rule.Host) - servicesInfo[serviceName] = sTmp - } - } else { - return nil, fmt.Errorf("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported") + if port.NodePort == 0 { + return nil, fmt.Errorf("service %s has no NodePort (only NodePort/LoadBalancer supported)", svc.Name) } + nodePort = port.NodePort + found = true break } } + if !found { + return nil, fmt.Errorf("service %s: port %d not found", svc.Name, path.Backend.Service.Port.Number) + } + + hInfo.Paths = append(hInfo.Paths, PathInfo{ + Path: path.Path, + Service: svc, + NodePort: int(nodePort), + NodeIps: nodeIps, + }) } + + hostsInfo[rule.Host] = hInfo } - return servicesInfo, nil + return hostsInfo, nil } diff --git a/internal/mocks/store.go b/internal/mocks/store.go index 446176f..7274e95 100644 --- a/internal/mocks/store.go +++ b/internal/mocks/store.go @@ -57,19 +57,19 @@ func (mr *MockStorerMockRecorder) GetIngress(key any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIngress", reflect.TypeOf((*MockStorer)(nil).GetIngress), key) } -// GetIngressServiceInfo mocks base method. -func (m *MockStorer) GetIngressServiceInfo(ingress *v10.Ingress) (map[string]store.ServiceInfo, error) { +// GetIngressHostsInfo mocks base method. +func (m *MockStorer) GetIngressHostsInfo(ingress *v10.Ingress) (map[string]store.HostInfo, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetIngressServiceInfo", ingress) - ret0, _ := ret[0].(map[string]store.ServiceInfo) + ret := m.ctrl.Call(m, "GetIngressHostsInfo", ingress) + ret0, _ := ret[0].(map[string]store.HostInfo) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetIngressServiceInfo indicates an expected call of GetIngressServiceInfo. -func (mr *MockStorerMockRecorder) GetIngressServiceInfo(ingress any) *gomock.Call { +// GetIngressHostsInfo indicates an expected call of GetIngressHostsInfo. +func (mr *MockStorerMockRecorder) GetIngressHostsInfo(ingress any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIngressServiceInfo", reflect.TypeOf((*MockStorer)(nil).GetIngressServiceInfo), ingress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIngressHostsInfo", reflect.TypeOf((*MockStorer)(nil).GetIngressHostsInfo), ingress) } // GetNodesIpList mocks base method. diff --git a/internal/service/loadbalancer/manager.go b/internal/service/loadbalancer/manager.go index 8eff12c..1c38b4c 100644 --- a/internal/service/loadbalancer/manager.go +++ b/internal/service/loadbalancer/manager.go @@ -145,59 +145,77 @@ func (m *Manager) GetIds() []string { func (m *Manager) TranslateIngressToLB(ingress *networkv1.Ingress, sslCerts map[string]string) (*serverscom.L7LoadBalancerCreateInput, error) { m.lock.Lock() defer m.lock.Unlock() - sInfo, err := m.store.GetIngressServiceInfo(ingress) + + hostsInfo, err := m.store.GetIngressHostsInfo(ingress) if err != nil { return nil, err } - var upstreamZones []serverscom.L7UpstreamZoneInput - var upstreams []serverscom.L7UpstreamInput var vhostZones []serverscom.L7VHostZoneInput - var locationZones []serverscom.L7LocationZoneInput - for sKey, service := range sInfo { - sslId := "" - sslEnabled := false + upstreamMap := make(map[string]serverscom.L7UpstreamZoneInput) + + for host, hInfo := range hostsInfo { + var locationZones []serverscom.L7LocationZoneInput vhostPorts := []int32{80} - upstreamId := fmt.Sprintf("upstream-zone-%s", sKey) - for _, ip := range service.NodeIps { - upstreams = append(upstreams, serverscom.L7UpstreamInput{ - IP: ip, - Weight: 1, - Port: int32(service.NodePort), - }) + sslEnabled := false + sslId := "" + + if id, ok := sslCerts[host]; ok { + sslId = id + sslEnabled = true + vhostPorts = []int32{443} } - for _, host := range service.Hosts { - if id, ok := sslCerts[host]; ok { - sslId = id - sslEnabled = true - vhostPorts = []int32{443} - } + vhostAnnotations := make(map[string]string) + for _, p := range hInfo.Paths { + upstreamId := fmt.Sprintf("upstream-zone-%s-%d", p.Service.Name, p.NodePort) + locationZones = append(locationZones, serverscom.L7LocationZoneInput{ - Location: "/", + Location: p.Path, UpstreamID: upstreamId, }) + + if _, ok := upstreamMap[upstreamId]; !ok { + var ups []serverscom.L7UpstreamInput + for _, ip := range p.NodeIps { + ups = append(ups, serverscom.L7UpstreamInput{ + IP: ip, + Port: int32(p.NodePort), + Weight: 1, + }) + } + upstream := serverscom.L7UpstreamZoneInput{ + ID: upstreamId, + Upstreams: ups, + } + upstream = *annotations.FillLBUpstreamZoneWithServiceAnnotations(&upstream, p.Service.Annotations) + upstreamMap[upstreamId] = upstream + } + + // last-win strategy for vhost annotations + for k, v := range p.Service.Annotations { + vhostAnnotations[k] = v + } } - vZInput := serverscom.L7VHostZoneInput{ - ID: fmt.Sprintf("vhost-zone-%s", sKey), - Domains: service.Hosts, + vz := serverscom.L7VHostZoneInput{ + ID: fmt.Sprintf("vhost-zone-%s", host), + Domains: []string{host}, SSLCertID: sslId, SSL: sslEnabled, Ports: vhostPorts, LocationZones: locationZones, } - vZInput = *annotations.FillLBVHostZoneWithServiceAnnotations(&vZInput, service.Annotations) - vhostZones = append(vhostZones, vZInput) + vz = *annotations.FillLBVHostZoneWithServiceAnnotations(&vz, vhostAnnotations) + vhostZones = append(vhostZones, vz) + } - uZInput := serverscom.L7UpstreamZoneInput{ - ID: upstreamId, - Upstreams: upstreams, - } - uZInput = *annotations.FillLBUpstreamZoneWithServiceAnnotations(&uZInput, service.Annotations) - upstreamZones = append(upstreamZones, uZInput) + var upstreamZones []serverscom.L7UpstreamZoneInput + for _, u := range upstreamMap { + upstreamZones = append(upstreamZones, u) } + if len(vhostZones) == 0 || len(upstreamZones) == 0 { return nil, errors.New("vhost or upstream can't be empty, can't continue") } diff --git a/internal/service/loadbalancer/manager_test.go b/internal/service/loadbalancer/manager_test.go index 871e6dd..ee11bde 100644 --- a/internal/service/loadbalancer/manager_test.go +++ b/internal/service/loadbalancer/manager_test.go @@ -12,6 +12,7 @@ import ( "github.com/serverscom/serverscom-ingress-controller/internal/mocks" "github.com/serverscom/serverscom-ingress-controller/internal/service/annotations" "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" networkv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -251,14 +252,59 @@ func TestTranslateIngressToLB(t *testing.T) { } sslCerts := map[string]string{ "example.com": "ssl-cert-id", + "foo.com": "ssl-cert-foo", } - serviceInfo := map[string]store.ServiceInfo{ - "service-key": { - Hosts: []string{"example.com"}, - NodePort: 30000, - NodeIps: []string{"192.168.1.1"}, - Annotations: map[string]string{annotations.LBBalancingAlgorithm: "round-robin"}, + hostsInfo := map[string]store.HostInfo{ + "example.com": { + Paths: []store.PathInfo{ + { + Path: "/api", + NodePort: 30000, + NodeIps: []string{"192.168.1.1"}, + Service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-key", + Annotations: map[string]string{annotations.LBBalancingAlgorithm: "round-robin"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 80, NodePort: 30000}}, + }, + }, + }, + { + Path: "/local", + NodePort: 30001, + NodeIps: []string{"192.168.1.1"}, + Service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-key2", + Annotations: map[string]string{annotations.LBBalancingAlgorithm: "least-connections"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 81, NodePort: 30001}}, + }, + }, + }, + }, + }, + "foo.com": { + Paths: []store.PathInfo{ + { + Path: "/", + NodePort: 30002, + NodeIps: []string{"192.168.1.2"}, + Service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-foo", + Annotations: map[string]string{annotations.LBBalancingAlgorithm: "round-robin"}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 80, NodePort: 30002}}, + }, + }, + }, + }, }, } @@ -271,22 +317,71 @@ func TestTranslateIngressToLB(t *testing.T) { t.Run("Translate ingress to lb input successfully", func(t *testing.T) { g := NewWithT(t) - storeHandler.EXPECT().GetIngressServiceInfo(ingress).Return(serviceInfo, nil) + storeHandler.EXPECT().GetIngressHostsInfo(ingress).Return(hostsInfo, nil) lbInput, err := manager.TranslateIngressToLB(ingress, sslCerts) g.Expect(err).To(BeNil()) g.Expect(lbInput).NotTo(BeNil()) + g.Expect(lbInput.VHostZones).To(HaveLen(2)) + + for _, vz := range lbInput.VHostZones { + expectedID := fmt.Sprintf("vhost-zone-%s", vz.Domains[0]) + g.Expect(vz.ID).To(Equal(expectedID)) + + g.Expect(vz.Domains).ToNot(BeEmpty()) + g.Expect(vz.LocationZones).ToNot(BeEmpty()) + g.Expect(vz.SSLCertID).ToNot(BeEmpty()) + + switch vz.Domains[0] { + case "example.com": + g.Expect(vz.SSLCertID).To(Equal("ssl-cert-id")) + g.Expect(vz.LocationZones).To(HaveLen(2)) + g.Expect(vz.LocationZones[0].Location).To(Equal("/api")) + g.Expect(vz.LocationZones[1].Location).To(Equal("/local")) + case "foo.com": + g.Expect(vz.SSLCertID).To(Equal("ssl-cert-foo")) + g.Expect(vz.LocationZones).To(HaveLen(1)) + g.Expect(vz.LocationZones[0].Location).To(Equal("/")) + default: + t.Fatalf("unexpected domain %s", vz.Domains[0]) + } + } + + expectedAlgorithmMethods := map[string]string{ + "upstream-zone-service-key-30000": "round-robin", + "upstream-zone-service-key2-30001": "least-connections", + "upstream-zone-service-foo-30002": "round-robin", + } + + g.Expect(lbInput.UpstreamZones).To(HaveLen(3)) + upstreamIDs := make(map[string]struct{}) + for _, uz := range lbInput.UpstreamZones { + if expected, ok := expectedAlgorithmMethods[uz.ID]; ok { + g.Expect(uz.Method).ToNot(BeNil()) + g.Expect(*uz.Method).To(Equal(expected)) + } + upstreamIDs[uz.ID] = struct{}{} + for _, u := range uz.Upstreams { + g.Expect([]string{"192.168.1.1", "192.168.1.2"}).To(ContainElement(u.IP)) + g.Expect(u.Weight).To(Equal(int32(1))) + } + } + for _, host := range hostsInfo { + for _, p := range host.Paths { + upID := fmt.Sprintf("upstream-zone-%s-%d", p.Service.Name, p.NodePort) + _, exists := upstreamIDs[upID] + g.Expect(exists).To(BeTrue(), "upstream %s should exist", upID) + } + } + expectedLBName := "ingress-a123" g.Expect(lbInput.Name).To(Equal(expectedLBName)) - g.Expect(lbInput.VHostZones[0].Domains).To(ConsistOf("example.com")) - g.Expect(lbInput.VHostZones[0].SSLCertID).To(Equal("ssl-cert-id")) - g.Expect(*lbInput.UpstreamZones[0].Method).To(Equal("round-robin")) g.Expect(*lbInput.Geoip).To(Equal(true)) }) t.Run("Services info fails", func(t *testing.T) { g := NewWithT(t) - storeHandler.EXPECT().GetIngressServiceInfo(ingress).Return(nil, errors.New("error")) + storeHandler.EXPECT().GetIngressHostsInfo(ingress).Return(nil, errors.New("error")) lbInput, err := manager.TranslateIngressToLB(ingress, sslCerts) g.Expect(err).To(HaveOccurred()) g.Expect(lbInput).To(BeNil()) @@ -294,7 +389,7 @@ func TestTranslateIngressToLB(t *testing.T) { t.Run("Services info is empty", func(t *testing.T) { g := NewWithT(t) - storeHandler.EXPECT().GetIngressServiceInfo(ingress).Return(make(map[string]store.ServiceInfo), nil) + storeHandler.EXPECT().GetIngressHostsInfo(ingress).Return(make(map[string]store.HostInfo), nil) lbInput, err := manager.TranslateIngressToLB(ingress, sslCerts) expectedErr := errors.New("vhost or upstream can't be empty, can't continue") g.Expect(err).To(Equal(expectedErr))