Merge pull request #318 from rramkumar1/refactors-for-testing

Break out some helper functions for better testing + reuse

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
 	}
 

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
+}

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)
+		}
+	}
+}

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}
 	}

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
+}

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)
+		}
+	}
+}