From d15f36f511562848f263c85c3c23662b81072c21 Mon Sep 17 00:00:00 2001 From: Rohit Ramkumar Date: Mon, 16 Apr 2018 15:44:51 -0700 Subject: [PATCH] Integrate ClusterServiceMapper into translator. --- pkg/backends/utils.go | 7 +- pkg/controller/controller.go | 49 +++++- pkg/controller/controller_test.go | 42 +++-- pkg/controller/errors/errors.go | 46 ----- pkg/controller/translator/translator.go | 174 +++---------------- pkg/controller/translator/translator_test.go | 32 ++++ pkg/controller/utils.go | 24 --- pkg/mapper/mapper.go | 4 +- pkg/mapper/mapper_test.go | 47 +---- pkg/test/manifests/ing1.yaml | 24 +++ pkg/test/utils.go | 43 +++++ pkg/utils/utils.go | 27 +++ 12 files changed, 231 insertions(+), 288 deletions(-) delete mode 100644 pkg/controller/errors/errors.go create mode 100644 pkg/test/manifests/ing1.yaml create mode 100644 pkg/test/utils.go diff --git a/pkg/backends/utils.go b/pkg/backends/utils.go index 37abdd533e..b80321650f 100644 --- a/pkg/backends/utils.go +++ b/pkg/backends/utils.go @@ -38,6 +38,7 @@ func ServicePorts(backendToService map[v1beta1.IngressBackend]v1.Service) (map[v svcPort, err := servicePort(ib, svc) if err != nil { result = multierror.Append(result, err) + continue } backendToServicePort[ib] = svcPort } @@ -49,7 +50,7 @@ func ServicePorts(backendToService map[v1beta1.IngressBackend]v1.Service) (map[v func servicePort(ib v1beta1.IngressBackend, svc v1.Service) (ServicePort, error) { // If service is not of type NodePort, return an error. if svc.Spec.Type != v1.ServiceTypeNodePort { - return ServicePort{}, fmt.Errorf("service %v is type %v, expected type NodePort", svc.Name, svc.Spec.Type) + return ServicePort{}, fmt.Errorf("service %v/%v for backend %+v is type %v, expected type NodePort", svc.Namespace, svc.Name, ib, svc.Spec.Type) } appProtocols, err := annotations.FromService(&svc).ApplicationProtocols() if err != nil { @@ -75,12 +76,12 @@ PortLoop: } if port == nil { - return ServicePort{}, fmt.Errorf("could not find matching port for backend %+v and service %s/%s. Looking for port %+v in %v", ib, svc.Namespace, ib.ServiceName, ib.ServicePort, svc.Spec.Ports) + return ServicePort{}, fmt.Errorf("could not find matching port on service %v/%v for backend %+v. Looking for port %+v in %v", svc.Namespace, ib.ServiceName, ib, ib.ServicePort, svc.Spec.Ports) } proto := annotations.ProtocolHTTP if protoStr, exists := appProtocols[port.Name]; exists { - glog.V(2).Infof("service %s/%s, port %q: using protocol to %q", svc.Namespace, ib.ServiceName, port, protoStr) + glog.V(2).Infof("service %v/%v, port %q: using protocol to %q", svc.Namespace, ib.ServiceName, port, protoStr) proto = annotations.AppProtocol(protoStr) } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index a0ecb247e9..74c9633a48 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -31,6 +31,8 @@ import ( unversionedcore "k8s.io/client-go/kubernetes/typed/core/v1" listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" + + multierror "github.com/hashicorp/go-multierror" "k8s.io/client-go/tools/record" "k8s.io/ingress-gce/pkg/annotations" @@ -39,6 +41,7 @@ import ( "k8s.io/ingress-gce/pkg/controller/translator" "k8s.io/ingress-gce/pkg/firewalls" "k8s.io/ingress-gce/pkg/loadbalancers" + "k8s.io/ingress-gce/pkg/mapper" "k8s.io/ingress-gce/pkg/tls" "k8s.io/ingress-gce/pkg/utils" ) @@ -165,12 +168,15 @@ func NewLoadBalancerController(ctx *context.ControllerContext, clusterManager *C if ctx.EndpointInformer != nil { endpointIndexer = ctx.EndpointInformer.GetIndexer() } + svcGetter := utils.SvcGetter{Store: ctx.ServiceInformer.GetStore()} lbc.Translator = translator.New(lbc.ctx, lbc.CloudClusterManager.ClusterNamer, ctx.ServiceInformer.GetIndexer(), ctx.NodeInformer.GetIndexer(), ctx.PodInformer.GetIndexer(), endpointIndexer, + mapper.NewClusterServiceMapper(svcGetter.Get, nil), negEnabled) + lbc.tlsLoader = &tls.TLSCertsFromSecretsLoader{Client: lbc.ctx.KubeClient} glog.V(3).Infof("Created new loadbalancer controller") @@ -242,7 +248,14 @@ func (lbc *LoadBalancerController) sync(key string) (retErr error) { } // gceNodePorts contains the ServicePorts used by only single-cluster ingress. - gceNodePorts := lbc.Translator.ToNodePorts(&gceIngresses) + var gceNodePorts []backends.ServicePort + for _, gceIngress := range gceIngresses.Items { + svcPortMapping, err := lbc.Translator.ServicePortMapping(&gceIngress) + if err != nil { + glog.Infof("%v", err.Error()) + } + gceNodePorts = append(gceNodePorts, extractSvcPorts(svcPortMapping)...) + } nodeNames, err := getReadyNodeNames(listers.NewNodeLister(lbc.nodeLister)) if err != nil { return err @@ -282,7 +295,24 @@ func (lbc *LoadBalancerController) sync(key string) (retErr error) { } func (lbc *LoadBalancerController) ensureIngress(key string, ing *extensions.Ingress, nodeNames []string, gceNodePorts []backends.ServicePort) error { - ingNodePorts := lbc.Translator.IngressToNodePorts(ing) + // Given an ingress, returns a mapping of IngressBackend -> ServicePort + svcPortMapping, err := lbc.Translator.ServicePortMapping(ing) + if err != nil { + // TODO(rramkumar): Clean this up, it's very ugly. + switch err.(type) { + case *multierror.Error: + // Emit an event for each error in the multierror. + merr := err.(*multierror.Error) + for _, e := range merr.Errors { + msg := fmt.Sprintf("%v", e) + lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, "Service", msg) + } + default: + msg := fmt.Sprintf("%v", err) + lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, "Service", msg) + } + } + ingNodePorts := extractSvcPorts(svcPortMapping) igs, err := lbc.CloudClusterManager.EnsureInstanceGroupsAndPorts(nodeNames, ingNodePorts) if err != nil { return err @@ -345,17 +375,17 @@ func (lbc *LoadBalancerController) ensureIngress(key string, ing *extensions.Ing return fmt.Errorf("unable to get loadbalancer: %v", err) } - urlMap, err := lbc.Translator.ToURLMap(ing) + urlMap, err := lbc.Translator.ToURLMap(ing, svcPortMapping) if err != nil { - return fmt.Errorf("convert to URL Map error %v", err) + return fmt.Errorf("error converting to URLMap: %v", err) } if err := l7.UpdateUrlMap(urlMap); err != nil { - return fmt.Errorf("update URL Map error: %v", err) + return fmt.Errorf("error updating URLMap: %v", err) } if err := lbc.updateIngressStatus(l7, ing); err != nil { - return fmt.Errorf("update ingress status error: %v", err) + return fmt.Errorf("error updating ingress status: %v", err) } return nil @@ -441,3 +471,10 @@ func updateAnnotations(client kubernetes.Interface, name, namespace string, anno } return nil } + +func extractSvcPorts(svcPortMapping map[extensions.IngressBackend]backends.ServicePort) (svcPorts []backends.ServicePort) { + for _, svcPort := range svcPortMapping { + svcPorts = append(svcPorts, svcPort) + } + return svcPorts +} diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 373192cce6..364385c43b 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -178,30 +178,42 @@ func newPortManager(st, end int, namer *utils.Namer) *nodePortManager { return &nodePortManager{map[string]int{}, st, end, namer} } +// service is a helper function to create k8s service object. +func service(ib *extensions.IngressBackend, namespace string, pm *nodePortManager) *api_v1.Service { + svc := &api_v1.Service{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: ib.ServiceName, + Namespace: namespace, + }, + } + var svcPort api_v1.ServicePort + switch ib.ServicePort.Type { + case intstr.Int: + svcPort = api_v1.ServicePort{Port: ib.ServicePort.IntVal} + default: + svcPort = api_v1.ServicePort{Name: ib.ServicePort.StrVal} + } + svcPort.NodePort = int32(pm.getNodePort(ib.ServiceName)) + svc.Spec.Ports = []api_v1.ServicePort{svcPort} + svc.Spec.Type = api_v1.ServiceTypeNodePort + return svc +} + // addIngress adds an ingress to the loadbalancer controllers ingress store. If // a nodePortManager is supplied, it also adds all backends to the service store // with a nodePort acquired through it. func addIngress(lbc *LoadBalancerController, ing *extensions.Ingress, pm *nodePortManager) { for _, rule := range ing.Spec.Rules { for _, path := range rule.HTTP.Paths { - svc := &api_v1.Service{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: path.Backend.ServiceName, - Namespace: ing.Namespace, - }, - } - var svcPort api_v1.ServicePort - switch path.Backend.ServicePort.Type { - case intstr.Int: - svcPort = api_v1.ServicePort{Port: path.Backend.ServicePort.IntVal} - default: - svcPort = api_v1.ServicePort{Name: path.Backend.ServicePort.StrVal} - } - svcPort.NodePort = int32(pm.getNodePort(path.Backend.ServiceName)) - svc.Spec.Ports = []api_v1.ServicePort{svcPort} + svc := service(&path.Backend, ing.Namespace, pm) lbc.ctx.ServiceInformer.GetIndexer().Add(svc) } } + if ing.Spec.Backend != nil { + defaultBackend := ing.Spec.Backend + defaultBackendSvc := service(defaultBackend, ing.Namespace, pm) + lbc.ctx.ServiceInformer.GetIndexer().Add(defaultBackendSvc) + } lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace).Create(ing) lbc.ctx.IngressInformer.GetIndexer().Add(ing) } diff --git a/pkg/controller/errors/errors.go b/pkg/controller/errors/errors.go deleted file mode 100644 index 3e7bcbf383..0000000000 --- a/pkg/controller/errors/errors.go +++ /dev/null @@ -1,46 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package errors - -import ( - "fmt" - - "k8s.io/api/core/v1" - "k8s.io/api/extensions/v1beta1" - - "k8s.io/ingress-gce/pkg/annotations" -) - -// ErrNodePortNotFound is returned when a port was not found. -type ErrNodePortNotFound struct { - Backend v1beta1.IngressBackend - Err error -} - -func (e ErrNodePortNotFound) Error() string { - return fmt.Sprintf("Could not find nodeport for backend %+v: %v", e.Backend, e.Err) -} - -// ErrSvcAppProtosParsing is returned when the service is malformed. -type ErrSvcAppProtosParsing struct { - Svc *v1.Service - Err error -} - -func (e ErrSvcAppProtosParsing) Error() string { - return fmt.Sprintf("could not parse %v annotation on Service %v/%v, err: %v", annotations.ServiceApplicationProtocolKey, e.Svc.Namespace, e.Svc.Name, e.Err) -} diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go index 297acb6764..266575b8fd 100644 --- a/pkg/controller/translator/translator.go +++ b/pkg/controller/translator/translator.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" listers "k8s.io/client-go/listers/core/v1" @@ -37,8 +36,8 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/backends" - "k8s.io/ingress-gce/pkg/controller/errors" "k8s.io/ingress-gce/pkg/loadbalancers" + "k8s.io/ingress-gce/pkg/mapper" "k8s.io/ingress-gce/pkg/utils" ) @@ -47,7 +46,7 @@ type recorderSource interface { } // New returns a new ControllerContext. -func New(recorders recorderSource, namer *utils.Namer, svcLister cache.Indexer, nodeLister cache.Indexer, podLister cache.Indexer, endpointLister cache.Indexer, negEnabled bool) *GCE { +func New(recorders recorderSource, namer *utils.Namer, svcLister cache.Indexer, nodeLister cache.Indexer, podLister cache.Indexer, endpointLister cache.Indexer, svcMapper mapper.ClusterServiceMapper, negEnabled bool) *GCE { return &GCE{ recorders, namer, @@ -55,6 +54,7 @@ func New(recorders recorderSource, namer *utils.Namer, svcLister cache.Indexer, nodeLister, podLister, endpointLister, + svcMapper, negEnabled, } } @@ -68,34 +68,25 @@ type GCE struct { nodeLister cache.Indexer podLister cache.Indexer endpointLister cache.Indexer + svcMapper mapper.ClusterServiceMapper negEnabled bool } // ToURLMap converts an ingress to a map of subdomain: url-regex: gce backend. -func (t *GCE) ToURLMap(ing *extensions.Ingress) (utils.GCEURLMap, error) { - hostPathBackend := utils.GCEURLMap{} +func (t *GCE) ToURLMap(ing *extensions.Ingress, svcPorts map[extensions.IngressBackend]backends.ServicePort) (utils.GCEURLMap, error) { + urlMap := utils.GCEURLMap{} for _, rule := range ing.Spec.Rules { if rule.HTTP == nil { - glog.Errorf("Ignoring non http Ingress rule") continue } pathToBackend := map[string]string{} for _, p := range rule.HTTP.Paths { - backendName, err := t.toGCEBackendName(&p.Backend, ing.Namespace) - if err != nil { - // If a service doesn't have a nodeport we can still forward traffic - // to all other services under the assumption that the user will - // modify nodeport. - if _, ok := err.(errors.ErrNodePortNotFound); ok { - t.recorders.Recorder(ing.Namespace).Eventf(ing, api_v1.EventTypeWarning, "Service", err.(errors.ErrNodePortNotFound).Error()) - continue - } - - // If a service doesn't have a backend, there's nothing the user - // can do to correct this (the admin might've limited quota). - // So keep requeuing the l7 till all backends exist. - return utils.GCEURLMap{}, err + // Get the corresponding ServicePort for this backend. + svcPort, ok := svcPorts[p.Backend] + if !ok { + return utils.GCEURLMap{}, fmt.Errorf("Could not find service for backend %+v", p.Backend) } + backendName := t.namer.Backend(svcPort.NodePort) // The Ingress spec defines empty path as catch-all, so if a user // asks for a single host and multiple empty paths, all traffic is // sent to one of the last backend in the rules list. @@ -110,145 +101,32 @@ func (t *GCE) ToURLMap(ing *extensions.Ingress) (utils.GCEURLMap, error) { if host == "" { host = loadbalancers.DefaultHost } - hostPathBackend[host] = pathToBackend + urlMap[host] = pathToBackend } var defaultBackendName string if ing.Spec.Backend != nil { - var err error - defaultBackendName, err = t.toGCEBackendName(ing.Spec.Backend, ing.Namespace) - if err != nil { - msg := fmt.Sprintf("%v", err) - if _, ok := err.(errors.ErrNodePortNotFound); ok { - msg = fmt.Sprintf("couldn't find nodeport for %v/%v", ing.Namespace, ing.Spec.Backend.ServiceName) - } - msg = fmt.Sprintf("failed to identify user specified default backend, %v, using system default", msg) - t.recorders.Recorder(ing.Namespace).Eventf(ing, api_v1.EventTypeWarning, "Service", msg) - } else if defaultBackendName != "" { - port, _ := t.namer.BackendPort(defaultBackendName) - msg := fmt.Sprintf("default backend set to %v:%v", ing.Spec.Backend.ServiceName, port) - t.recorders.Recorder(ing.Namespace).Eventf(ing, api_v1.EventTypeNormal, "Service", msg) + svcPort, ok := svcPorts[*ing.Spec.Backend] + if ok { + defaultBackendName = t.namer.Backend(svcPort.NodePort) } - } else { - t.recorders.Recorder(ing.Namespace).Eventf(ing, api_v1.EventTypeNormal, "Service", "no user specified default backend, using system default") + // In the case where we could not find the ServicePort for the + // default backend, we will use our default (see loadbalancer/l7.go) } - hostPathBackend.PutDefaultBackendName(defaultBackendName) - return hostPathBackend, nil + urlMap.PutDefaultBackendName(defaultBackendName) + return urlMap, nil } -func (t *GCE) toGCEBackendName(be *extensions.IngressBackend, ns string) (string, error) { - if be == nil { - return "", nil - } - port, err := t.getServiceNodePort(*be, ns) +// ServicePortMapping converts an Ingress to a IngressBackend -> ServicePort mapping. +func (t *GCE) ServicePortMapping(ing *extensions.Ingress) (map[extensions.IngressBackend]backends.ServicePort, error) { + backendToServiceMap, err := t.svcMapper.Services(ing) if err != nil { - return "", err - } - backendName := t.namer.Backend(port.NodePort) - return backendName, nil -} - -// getServiceNodePort looks in the svc store for a matching service:port, -// and returns the nodeport. -func (t *GCE) getServiceNodePort(be extensions.IngressBackend, namespace string) (backends.ServicePort, error) { - obj, exists, err := t.svcLister.Get( - &api_v1.Service{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: be.ServiceName, - Namespace: namespace, - }, - }) - if !exists { - return backends.ServicePort{}, errors.ErrNodePortNotFound{ - Backend: be, - Err: fmt.Errorf("service %v/%v not found in store", namespace, be.ServiceName), - } - } - if err != nil { - return backends.ServicePort{}, errors.ErrNodePortNotFound{Backend: be, Err: err} + return nil, err } - svc := obj.(*api_v1.Service) - appProtocols, err := annotations.FromService(svc).ApplicationProtocols() + backendToServicePortsMap, err := backends.ServicePorts(backendToServiceMap) if err != nil { - return backends.ServicePort{}, errors.ErrSvcAppProtosParsing{Svc: svc, Err: err} - } - - var port *api_v1.ServicePort -PortLoop: - for _, p := range svc.Spec.Ports { - np := p - switch be.ServicePort.Type { - case intstr.Int: - if p.Port == be.ServicePort.IntVal { - port = &np - break PortLoop - } - default: - if p.Name == be.ServicePort.StrVal { - port = &np - break PortLoop - } - } - } - - if port == nil { - return backends.ServicePort{}, errors.ErrNodePortNotFound{ - Backend: be, - Err: fmt.Errorf("could not find matching nodeport from service"), - } - } - - proto := annotations.ProtocolHTTP - if protoStr, exists := appProtocols[port.Name]; exists { - proto = annotations.AppProtocol(protoStr) - } - - p := backends.ServicePort{ - NodePort: int64(port.NodePort), - Protocol: proto, - SvcName: types.NamespacedName{Namespace: namespace, Name: be.ServiceName}, - SvcPort: be.ServicePort, - SvcTargetPort: port.TargetPort.String(), - NEGEnabled: t.negEnabled && annotations.FromService(svc).NEGEnabled(), - } - return p, nil -} - -// ToNodePorts is a helper method over IngressToNodePorts to process a list of ingresses. -func (t *GCE) ToNodePorts(ings *extensions.IngressList) []backends.ServicePort { - var knownPorts []backends.ServicePort - for _, ing := range ings.Items { - knownPorts = append(knownPorts, t.IngressToNodePorts(&ing)...) - } - return knownPorts -} - -// IngressToNodePorts converts a pathlist to a flat list of nodeports for the given ingress. -func (t *GCE) IngressToNodePorts(ing *extensions.Ingress) []backends.ServicePort { - var knownPorts []backends.ServicePort - defaultBackend := ing.Spec.Backend - if defaultBackend != nil { - port, err := t.getServiceNodePort(*defaultBackend, ing.Namespace) - if err != nil { - glog.Infof("%v", err) - } else { - knownPorts = append(knownPorts, port) - } - } - for _, rule := range ing.Spec.Rules { - if rule.HTTP == nil { - glog.Errorf("ignoring non http Ingress rule") - continue - } - for _, path := range rule.HTTP.Paths { - port, err := t.getServiceNodePort(path.Backend, ing.Namespace) - if err != nil { - glog.Infof("%v", err) - continue - } - knownPorts = append(knownPorts, port) - } + return nil, err } - return knownPorts + return backendToServicePortsMap, nil } func getZone(n *api_v1.Node) string { diff --git a/pkg/controller/translator/translator_test.go b/pkg/controller/translator/translator_test.go index 8d378c14d7..599213eb01 100644 --- a/pkg/controller/translator/translator_test.go +++ b/pkg/controller/translator/translator_test.go @@ -18,6 +18,7 @@ package translator import ( "fmt" + "reflect" "testing" "time" @@ -32,9 +33,12 @@ import ( unversionedcore "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/record" + "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/backends" "k8s.io/ingress-gce/pkg/context" + "k8s.io/ingress-gce/pkg/mapper" + "k8s.io/ingress-gce/pkg/test" "k8s.io/ingress-gce/pkg/utils" ) @@ -53,12 +57,14 @@ func gceForTest(negEnabled bool) *GCE { namer := utils.NewNamer("uid1", "fw1") ctx := context.NewControllerContext(client, nil, apiv1.NamespaceAll, 1*time.Second, negEnabled) + svcGetter := utils.SvcGetter{Store: ctx.ServiceInformer.GetStore()} gce := &GCE{ recorders: ctx, namer: namer, svcLister: ctx.ServiceInformer.GetIndexer(), nodeLister: ctx.NodeInformer.GetIndexer(), podLister: ctx.PodInformer.GetIndexer(), + svcMapper: mapper.NewClusterServiceMapper(svcGetter.Get, nil), negEnabled: negEnabled, } if ctx.EndpointInformer != nil { @@ -67,6 +73,32 @@ func gceForTest(negEnabled bool) *GCE { return gce } +func TestToURLMap(t *testing.T) { + ing, err := test.GetTestIngress("../../test/manifests/ing1.yaml") + if err != nil { + t.Fatalf("Error occured when getting test Ingress: %v", err) + } + translator := gceForTest(false) + backendToServicePortMap := map[v1beta1.IngressBackend]backends.ServicePort{ + v1beta1.IngressBackend{ServiceName: "default", ServicePort: intstr.FromInt(80)}: backends.ServicePort{NodePort: 30000}, + v1beta1.IngressBackend{ServiceName: "testy", ServicePort: intstr.FromInt(80)}: backends.ServicePort{NodePort: 30001}, + v1beta1.IngressBackend{ServiceName: "testx", ServicePort: intstr.FromInt(80)}: backends.ServicePort{NodePort: 30002}, + v1beta1.IngressBackend{ServiceName: "testz", ServicePort: intstr.FromInt(80)}: backends.ServicePort{NodePort: 30003}, + } + expectedUrlMap := utils.GCEURLMap{ + "foo.bar.com": { + "/foo": translator.namer.Backend(30002), + "/bar": translator.namer.Backend(30001), + "/baz": translator.namer.Backend(30003), + }, + } + expectedUrlMap.PutDefaultBackendName(translator.namer.Backend(30000)) + urlMap, _ := translator.ToURLMap(&ing, backendToServicePortMap) + if !reflect.DeepEqual(expectedUrlMap, urlMap) { + t.Errorf("Result %v does not match expected %v", urlMap, expectedUrlMap) + } +} + func TestGetProbe(t *testing.T) { translator := gceForTest(false) nodePortToHealthCheck := map[backends.ServicePort]string{ diff --git a/pkg/controller/utils.go b/pkg/controller/utils.go index 7e648e2220..ee4353379a 100644 --- a/pkg/controller/utils.go +++ b/pkg/controller/utils.go @@ -214,27 +214,3 @@ func nodeStatusChanged(old, cur *api_v1.Node) bool { } return false } - -// Encapsulates an object that can get a service from a cluster. -type SvcGetter struct { - cache.Store -} - -func (s *SvcGetter) Get(svcName, namespace string) (*api_v1.Service, error) { - obj, exists, err := s.Store.Get( - &api_v1.Service{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: svcName, - Namespace: namespace, - }, - }, - ) - if !exists { - return nil, fmt.Errorf("service %v/%v not found in store", namespace, svcName) - } - if err != nil { - return nil, err - } - svc := obj.(*api_v1.Service) - return svc, nil -} diff --git a/pkg/mapper/mapper.go b/pkg/mapper/mapper.go index 0836176294..85fd591aa8 100644 --- a/pkg/mapper/mapper.go +++ b/pkg/mapper/mapper.go @@ -62,7 +62,7 @@ func (c *clusterServiceMapper) Services(ing *v1beta1.Ingress) (map[v1beta1.Ingre } svc, err := c.svcGetter(defaultBackend.ServiceName, ing.Namespace) if err != nil { - result = multierror.Append(result, fmt.Errorf("error getting %v/%v: %v", defaultBackend.ServiceName, ing.Namespace, err)) + result = multierror.Append(result, fmt.Errorf("error getting service %v/%v for backend %+v: %v", ing.Namespace, defaultBackend.ServiceName, *defaultBackend, err)) } else { backendToService[*defaultBackend] = *svc } @@ -82,7 +82,7 @@ Loop: } svc, err := c.svcGetter(path.Backend.ServiceName, ing.Namespace) if err != nil { - result = multierror.Append(result, fmt.Errorf("error getting %v/%v", ing.Namespace, path.Backend.ServiceName)) + result = multierror.Append(result, fmt.Errorf("error getting service %v/%v for backend %+v: %v", ing.Namespace, path.Backend.ServiceName, path.Backend, err)) continue } backendToService[path.Backend] = *svc diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go index 9abfa1ee2b..9b1e034e93 100644 --- a/pkg/mapper/mapper_test.go +++ b/pkg/mapper/mapper_test.go @@ -22,43 +22,14 @@ import ( "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - utilyaml "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/ingress-gce/pkg/test" ) -var rawIng = ` -apiVersion: extensions/v1beta1 -kind: Ingress -metadata: - name: test -spec: - backend: - serviceName: default - servicePort: 80 - rules: - - host: foo.bar.com - http: - paths: - - path: /foo - backend: - serviceName: testx - servicePort: 80 - - path: /bar - backend: - serviceName: testy - servicePort: 80 - - path: /baz - backend: - serviceName: testz - servicePort: 80 -` - func TestServices(t *testing.T) { - ing, err := getTestIngress() + ing, err := test.GetTestIngress("../test/manifests/ing1.yaml") if err != nil { - t.Errorf("Error occured when constructing test Ingress: %v", err) + t.Fatalf("Error occured when getting test Ingress: %v", err) } testCases := []struct { @@ -103,15 +74,3 @@ func stubbedErrorSvcGetter(svcName, namespace string) (*v1.Service, error) { func stubbedSvcGetter(svcName, namespace string) (*v1.Service, error) { return &v1.Service{ObjectMeta: meta_v1.ObjectMeta{Name: svcName}}, nil } - -func getTestIngress() (v1beta1.Ingress, error) { - ing := v1beta1.Ingress{} - json, err := utilyaml.ToJSON([]byte(rawIng)) - if err != nil { - return v1beta1.Ingress{}, err - } - if err := runtime.DecodeInto(legacyscheme.Codecs.UniversalDecoder(), json, &ing); err != nil { - return v1beta1.Ingress{}, err - } - return ing, nil -} diff --git a/pkg/test/manifests/ing1.yaml b/pkg/test/manifests/ing1.yaml new file mode 100644 index 0000000000..17af226e1d --- /dev/null +++ b/pkg/test/manifests/ing1.yaml @@ -0,0 +1,24 @@ +apiVersion: extensions/v1beta1 +kind: Ingress +metadata: + name: test +spec: + backend: + serviceName: default + servicePort: 80 + rules: + - host: foo.bar.com + http: + paths: + - path: /foo + backend: + serviceName: testx + servicePort: 80 + - path: /bar + backend: + serviceName: testy + servicePort: 80 + - path: /baz + backend: + serviceName: testz + servicePort: 80 diff --git a/pkg/test/utils.go b/pkg/test/utils.go new file mode 100644 index 0000000000..8269202237 --- /dev/null +++ b/pkg/test/utils.go @@ -0,0 +1,43 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "io/ioutil" + + "k8s.io/api/extensions/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + utilyaml "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/kubernetes/pkg/api/legacyscheme" +) + +// GetTestIngress returns an Ingress based on a spec defined in a YAML file. +func GetTestIngress(filename string) (v1beta1.Ingress, error) { + ing := v1beta1.Ingress{} + rawIng, err := ioutil.ReadFile(filename) + if err != nil { + return ing, err + } + json, err := utilyaml.ToJSON([]byte(rawIng)) + if err != nil { + return v1beta1.Ingress{}, err + } + if err := runtime.DecodeInto(legacyscheme.Codecs.UniversalDecoder(), json, &ing); err != nil { + return v1beta1.Ingress{}, err + } + return ing, nil +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 12c907c688..be7f1c005e 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -22,6 +22,9 @@ import ( "strings" "google.golang.org/api/googleapi" + api_v1 "k8s.io/api/core/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" ) const ( @@ -137,6 +140,30 @@ func trimFieldsEvenly(max int, fields ...string) []string { return ret } +// Encapsulates an object that can get a service from a cluster. +type SvcGetter struct { + cache.Store +} + +func (s *SvcGetter) Get(svcName, namespace string) (*api_v1.Service, error) { + obj, exists, err := s.Store.Get( + &api_v1.Service{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: svcName, + Namespace: namespace, + }, + }, + ) + if !exists { + return nil, fmt.Errorf("service %v/%v not found in store", namespace, svcName) + } + if err != nil { + return nil, err + } + svc := obj.(*api_v1.Service) + return svc, nil +} + // BackendServiceRelativeResourcePath returns a relative path of the link for a // BackendService given its name. func BackendServiceRelativeResourcePath(name string) string {