From 749f086d251d3510bda7b7ec8ff15358d19d6a00 Mon Sep 17 00:00:00 2001 From: Rohit Ramkumar Date: Thu, 7 Jun 2018 10:30:33 -0700 Subject: [PATCH] Break out some helper function for better testing + reuse --- pkg/backendconfig/backendconfig.go | 20 ++---- pkg/backendconfig/utils.go | 37 ++++++++++ pkg/backendconfig/utils_test.go | 74 ++++++++++++++++++++ pkg/controller/translator/translator.go | 19 +---- pkg/controller/translator/utils.go | 41 +++++++++++ pkg/controller/translator/utils_test.go | 92 +++++++++++++++++++++++++ 6 files changed, 250 insertions(+), 33 deletions(-) create mode 100644 pkg/backendconfig/utils.go create mode 100644 pkg/backendconfig/utils_test.go create mode 100644 pkg/controller/translator/utils.go create mode 100644 pkg/controller/translator/utils_test.go diff --git a/pkg/backendconfig/backendconfig.go b/pkg/backendconfig/backendconfig.go index a17c2758e5..172f7e53e3 100644 --- a/pkg/backendconfig/backendconfig.go +++ b/pkg/backendconfig/backendconfig.go @@ -18,7 +18,6 @@ package backendconfig import ( "fmt" - "strconv" "github.com/golang/glog" @@ -78,26 +77,17 @@ func GetServicesForBackendConfig(svcLister cache.Store, backendConfig *backendco // GetBackendConfigForServicePort returns the corresponding BackendConfig for // the given ServicePort if specified. func GetBackendConfigForServicePort(backendConfigLister cache.Store, svc *apiv1.Service, svcPort *apiv1.ServicePort) (*backendconfigv1beta1.BackendConfig, error) { - backendConfigNames, err := annotations.FromService(svc).GetBackendConfigs() + backendConfigs, err := annotations.FromService(svc).GetBackendConfigs() if err != nil { - return nil, fmt.Errorf("failed to get BackendConfig names from service %s/%s: %v", svc.Namespace, svc.Name, err) + return nil, fmt.Errorf("failed to get BackendConfigs from service %s/%s: %v", svc.Namespace, svc.Name, err) } // BackendConfig is optional for ServicePort. - if backendConfigNames == nil { + if backendConfigs == nil { return nil, nil } - // Both port name and port number are allowed. - configName := "" - if name, ok := backendConfigNames.Ports[svcPort.Name]; ok { - configName = name - } else if name, ok := backendConfigNames.Ports[strconv.Itoa(int(svcPort.Port))]; ok { - configName = name - } else if len(backendConfigNames.Default) != 0 { - configName = backendConfigNames.Default - } - // Return earlier if no BackendConfig matches. - if len(configName) == 0 { + configName := BackendConfigName(*backendConfigs, *svcPort) + if configName == "" { return nil, nil } diff --git a/pkg/backendconfig/utils.go b/pkg/backendconfig/utils.go new file mode 100644 index 0000000000..9f8ac3ff05 --- /dev/null +++ b/pkg/backendconfig/utils.go @@ -0,0 +1,37 @@ +/* +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 backendconfig + +import ( + "strconv" + + apiv1 "k8s.io/api/core/v1" + + "k8s.io/ingress-gce/pkg/annotations" +) + +// BackendConfigName returns the name of the BackendConfig which is associated +// with the given ServicePort. +func BackendConfigName(backendConfigs annotations.BackendConfigs, svcPort apiv1.ServicePort) string { + configName := "" + // Both port name and port number are allowed. + if name, ok := backendConfigs.Ports[svcPort.Name]; ok { + configName = name + } else if name, ok := backendConfigs.Ports[strconv.Itoa(int(svcPort.Port))]; ok { + configName = name + } else if len(backendConfigs.Default) != 0 { + configName = backendConfigs.Default + } + return configName +} diff --git a/pkg/backendconfig/utils_test.go b/pkg/backendconfig/utils_test.go new file mode 100644 index 0000000000..248ca31b85 --- /dev/null +++ b/pkg/backendconfig/utils_test.go @@ -0,0 +1,74 @@ +/* +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 backendconfig + +import ( + "testing" + + apiv1 "k8s.io/api/core/v1" + + "k8s.io/ingress-gce/pkg/annotations" +) + +func TestBackendConfigName(t *testing.T) { + testCases := []struct { + desc string + backendConfigs annotations.BackendConfigs + svcPort apiv1.ServicePort + expected string + }{ + { + desc: "matched on port name", + backendConfigs: annotations.BackendConfigs{ + Ports: map[string]string{"http": "config-http"}, + Default: "default", + }, + svcPort: apiv1.ServicePort{Name: "http"}, + expected: "config-http", + }, + { + desc: "matched on port number", + backendConfigs: annotations.BackendConfigs{ + Ports: map[string]string{"80": "config-http"}, + Default: "default", + }, + svcPort: apiv1.ServicePort{Name: "foo", Port: 80}, + expected: "config-http", + }, + { + desc: "matched on default", + backendConfigs: annotations.BackendConfigs{ + Ports: map[string]string{"http": "config-http"}, + Default: "default", + }, + svcPort: apiv1.ServicePort{Name: "https", Port: 443}, + expected: "default", + }, + { + desc: "no match", + backendConfigs: annotations.BackendConfigs{ + Ports: map[string]string{"http": "config-http"}, + }, + svcPort: apiv1.ServicePort{Name: "https", Port: 443}, + expected: "", + }, + } + + for _, tc := range testCases { + result := BackendConfigName(tc.backendConfigs, tc.svcPort) + if result != tc.expected { + t.Errorf("%s: expected %s but got %s", tc.desc, tc.expected, result) + } + } +} diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go index 3a30605c01..da2e08bc61 100644 --- a/pkg/controller/translator/translator.go +++ b/pkg/controller/translator/translator.go @@ -72,24 +72,7 @@ func (t *Translator) getServicePort(id utils.ServicePortID) (*utils.ServicePort, return nil, errors.ErrSvcAppProtosParsing{Svc: svc, Err: err} } - var port *api_v1.ServicePort -PortLoop: - for _, p := range svc.Spec.Ports { - np := p - switch id.Port.Type { - case intstr.Int: - if p.Port == id.Port.IntVal { - port = &np - break PortLoop - } - default: - if p.Name == id.Port.StrVal { - port = &np - break PortLoop - } - } - } - + port := ServicePort(*svc, id.Port) if port == nil { return nil, errors.ErrSvcPortNotFound{ServicePortID: id} } diff --git a/pkg/controller/translator/utils.go b/pkg/controller/translator/utils.go new file mode 100644 index 0000000000..3ee011f1ab --- /dev/null +++ b/pkg/controller/translator/utils.go @@ -0,0 +1,41 @@ +/* +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 translator + +import ( + api_v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// ServicePort is a helper function that retrieves a port of a Service. +func ServicePort(svc api_v1.Service, port intstr.IntOrString) *api_v1.ServicePort { + var svcPort *api_v1.ServicePort +PortLoop: + for _, p := range svc.Spec.Ports { + np := p + switch port.Type { + case intstr.Int: + if p.Port == port.IntVal { + svcPort = &np + break PortLoop + } + default: + if p.Name == port.StrVal { + svcPort = &np + break PortLoop + } + } + } + return svcPort +} diff --git a/pkg/controller/translator/utils_test.go b/pkg/controller/translator/utils_test.go new file mode 100644 index 0000000000..58408069ad --- /dev/null +++ b/pkg/controller/translator/utils_test.go @@ -0,0 +1,92 @@ +/* +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 translator + +import ( + "reflect" + "testing" + + api_v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +var ( + testSvc = api_v1.Service{ + Spec: api_v1.ServiceSpec{ + Ports: []api_v1.ServicePort{ + { + Name: "foo", + Port: 1000, + }, + { + Name: "bar", + Port: 1001, + }, + { + Name: "baz", + Port: 1002, + }, + { + Name: "qux", + Port: 1003, + }, + }, + }, + } +) + +func TestServicePort(t *testing.T) { + testCases := []struct { + desc string + port intstr.IntOrString + expected *api_v1.ServicePort + }{ + { + desc: "match on port number", + port: intstr.FromInt(1000), + expected: &api_v1.ServicePort{ + Name: "foo", + Port: 1000, + }, + }, + { + desc: "match on port name", + port: intstr.FromString("foo"), + expected: &api_v1.ServicePort{ + Name: "foo", + Port: 1000, + }, + }, + { + desc: "match on last port number", + port: intstr.FromInt(1003), + expected: &api_v1.ServicePort{ + Name: "qux", + Port: 1003, + }, + }, + { + desc: "no match", + port: intstr.FromInt(3000), + expected: nil, + }, + } + + for _, tc := range testCases { + result := ServicePort(testSvc, tc.port) + if !reflect.DeepEqual(result, tc.expected) { + t.Errorf("%s: expected %+v but got %+v", tc.desc, tc.expected, result) + } + } +}