From 9a351e3670ee19b0485d28728d81e5e1d83ab7bd Mon Sep 17 00:00:00 2001
From: Alex Robinson <arob@google.com>
Date: Fri, 3 Apr 2015 19:06:25 +0000
Subject: [PATCH] Move validation of load balancers only supporting TCP ports
 to validation.go.

---
 pkg/api/validation/validation.go              |  8 +++++
 pkg/api/validation/validation_test.go         | 31 +++++++++++++++++++
 .../servicecontroller/servicecontroller.go    |  1 -
 pkg/registry/service/rest.go                  | 22 -------------
 4 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
index 19de235a949..b49e883796b 100644
--- a/pkg/api/validation/validation.go
+++ b/pkg/api/validation/validation.go
@@ -927,6 +927,14 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
 		}
 	}
 
+	if service.Spec.CreateExternalLoadBalancer {
+		for i := range service.Spec.Ports {
+			if service.Spec.Ports[i].Protocol != api.ProtocolTCP {
+				allErrs = append(allErrs, errs.NewFieldInvalid("spec.ports", service.Spec.Ports[i], "cannot create an external load balancer with non-TCP ports"))
+			}
+		}
+	}
+
 	return allErrs
 }
 
diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go
index 8eda725f2ba..ddc27439946 100644
--- a/pkg/api/validation/validation_test.go
+++ b/pkg/api/validation/validation_test.go
@@ -1583,6 +1583,22 @@ func TestValidateService(t *testing.T) {
 			},
 			numErrs: 1,
 		},
+		{
+			name: "invalid load balancer protocol 1",
+			tweakSvc: func(s *api.Service) {
+				s.Spec.CreateExternalLoadBalancer = true
+				s.Spec.Ports[0].Protocol = "UDP"
+			},
+			numErrs: 1,
+		},
+		{
+			name: "invalid load balancer protocol 2",
+			tweakSvc: func(s *api.Service) {
+				s.Spec.CreateExternalLoadBalancer = true
+				s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345, Protocol: "UDP"})
+			},
+			numErrs: 1,
+		},
 		{
 			name: "valid 1",
 			tweakSvc: func(s *api.Service) {
@@ -1620,6 +1636,21 @@ func TestValidateService(t *testing.T) {
 			},
 			numErrs: 0,
 		},
+		{
+			name: "valid external load balancer",
+			tweakSvc: func(s *api.Service) {
+				s.Spec.CreateExternalLoadBalancer = true
+			},
+			numErrs: 0,
+		},
+		{
+			name: "valid external load balancer 2 ports",
+			tweakSvc: func(s *api.Service) {
+				s.Spec.CreateExternalLoadBalancer = true
+				s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345, Protocol: "TCP"})
+			},
+			numErrs: 0,
+		},
 	}
 
 	for _, tc := range testCases {
diff --git a/pkg/cloudprovider/servicecontroller/servicecontroller.go b/pkg/cloudprovider/servicecontroller/servicecontroller.go
index d77e6241d3e..9a4c98a0d2d 100644
--- a/pkg/cloudprovider/servicecontroller/servicecontroller.go
+++ b/pkg/cloudprovider/servicecontroller/servicecontroller.go
@@ -385,7 +385,6 @@ func (s *ServiceController) loadBalancerName(service *api.Service) string {
 	return s.cloud.GetLoadBalancerName(s.clusterName, service.Namespace, service.Name)
 }
 
-// TODO: Deduplicate this with the copy in pkg/registry/service/rest.go.
 func getTCPPorts(service *api.Service) ([]int, error) {
 	ports := []int{}
 	for i := range service.Spec.Ports {
diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go
index 5050eaa7096..fb5b1195ae8 100644
--- a/pkg/registry/service/rest.go
+++ b/pkg/registry/service/rest.go
@@ -94,14 +94,6 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err
 		return nil, err
 	}
 
-	// Make sure that we'll be able to create a load balancer for the service,
-	// even though it'll be created by the ServiceController.
-	if service.Spec.CreateExternalLoadBalancer {
-		if _, err := getTCPPorts(service); err != nil {
-			return nil, err
-		}
-	}
-
 	releaseServiceIP := false
 	defer func() {
 		if releaseServiceIP {
@@ -251,17 +243,3 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou
 	}
 	return nil, nil, fmt.Errorf("no endpoints available for %q", id)
 }
-
-// TODO: Deduplicate with the copy of this in pkg/registry/service/rest.go
-func getTCPPorts(service *api.Service) ([]int, error) {
-	ports := []int{}
-	for i := range service.Spec.Ports {
-		sp := &service.Spec.Ports[i]
-		if sp.Protocol != api.ProtocolTCP {
-			// TODO: Support UDP here too.
-			return nil, fmt.Errorf("external load balancers for non TCP services are not currently supported.")
-		}
-		ports = append(ports, sp.Port)
-	}
-	return ports, nil
-}