diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index f0835821703..13fdaf4bf30 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -81,7 +81,7 @@ type TCPLoadBalancer interface { // if so, what its status is. GetTCPLoadBalancer(name, region string) (status *api.LoadBalancerStatus, exists bool, err error) // CreateTCPLoadBalancer creates a new tcp load balancer. Returns the status of the balancer - CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) + CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []*api.ServicePort, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) // UpdateTCPLoadBalancer updates hosts under the specified load balancer. UpdateTCPLoadBalancer(name, region string, hosts []string) error // EnsureTCPLoadBalancerDeleted deletes the specified load balancer if it diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index 15ef13c1522..45ea2468b8a 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/pkg/cloudprovider/fake/fake.go @@ -33,7 +33,7 @@ type FakeBalancer struct { Name string Region string ExternalIP net.IP - Ports []int + Ports []*api.ServicePort Hosts []string } @@ -119,7 +119,7 @@ func (f *FakeCloud) GetTCPLoadBalancer(name, region string) (*api.LoadBalancerSt // CreateTCPLoadBalancer is a test-spy implementation of TCPLoadBalancer.CreateTCPLoadBalancer. // It adds an entry "create" into the internal method call record. -func (f *FakeCloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) { +func (f *FakeCloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []*api.ServicePort, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) { f.addCall("create") f.Balancers = append(f.Balancers, FakeBalancer{name, region, externalIP, ports, hosts}) diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index 38544416060..50ac1fbcc2c 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -326,7 +326,7 @@ func translateAffinityType(affinityType api.ServiceAffinity) GCEAffinityType { // CreateTCPLoadBalancer is an implementation of TCPLoadBalancer.CreateTCPLoadBalancer. // TODO(a-robinson): Don't just ignore specified IP addresses. Check if they're // owned by the project and available to be used, and use them if they are. -func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) { +func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []*api.ServicePort, hosts []string, affinityType api.ServiceAffinity) (*api.LoadBalancerStatus, error) { err := gce.makeTargetPool(name, region, hosts, translateAffinityType(affinityType)) if err != nil { if !isHTTPErrorCode(err, http.StatusConflict) { @@ -341,11 +341,11 @@ func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.I minPort := 65536 maxPort := 0 for i := range ports { - if ports[i] < minPort { - minPort = ports[i] + if ports[i].Port < minPort { + minPort = ports[i].Port } - if ports[i] > maxPort { - maxPort = ports[i] + if ports[i].Port > maxPort { + maxPort = ports[i].Port } } req := &compute.ForwardingRule{ diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 53db708f1a1..2415114bddb 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -495,7 +495,7 @@ func (lb *LoadBalancer) GetTCPLoadBalancer(name, region string) (*api.LoadBalanc // a list of regions (from config) and query/create loadbalancers in // each region. -func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinity api.ServiceAffinity) (*api.LoadBalancerStatus, error) { +func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []*api.ServicePort, hosts []string, affinity api.ServiceAffinity) (*api.LoadBalancerStatus, error) { glog.V(4).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, ports, hosts, affinity) if len(ports) > 1 { @@ -534,7 +534,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne _, err = members.Create(lb.network, members.CreateOpts{ PoolID: pool.ID, - ProtocolPort: ports[0], //TODO: need to handle multi-port + ProtocolPort: ports[0].Port, //TODO: need to handle multi-port Address: addr, }).Extract() if err != nil { @@ -569,7 +569,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne Description: fmt.Sprintf("Kubernetes external service %s", name), Address: externalIP.String(), Protocol: "TCP", - ProtocolPort: ports[0], //TODO: need to handle multi-port + ProtocolPort: ports[0].Port, //TODO: need to handle multi-port PoolID: pool.ID, Persistence: persistence, }).Extract() diff --git a/pkg/cloudprovider/servicecontroller/servicecontroller.go b/pkg/cloudprovider/servicecontroller/servicecontroller.go index 0acdd93b136..3e4d5e9d099 100644 --- a/pkg/cloudprovider/servicecontroller/servicecontroller.go +++ b/pkg/cloudprovider/servicecontroller/servicecontroller.go @@ -331,7 +331,7 @@ func (s *ServiceController) persistUpdate(service *api.Service) error { } func (s *ServiceController) createExternalLoadBalancer(service *api.Service) error { - ports, err := getTCPPorts(service) + ports, err := getPortsForLB(service) if err != nil { return err } @@ -436,7 +436,7 @@ func needsUpdate(oldService *api.Service, newService *api.Service) bool { if wantsExternalLoadBalancer(oldService) != wantsExternalLoadBalancer(newService) { return true } - if !portsEqual(oldService, newService) || oldService.Spec.SessionAffinity != newService.Spec.SessionAffinity { + if !portsEqualForLB(oldService, newService) || oldService.Spec.SessionAffinity != newService.Spec.SessionAffinity { return true } if len(oldService.Spec.DeprecatedPublicIPs) != len(newService.Spec.DeprecatedPublicIPs) { @@ -454,8 +454,8 @@ func (s *ServiceController) loadBalancerName(service *api.Service) string { return cloudprovider.GetLoadBalancerName(service) } -func getTCPPorts(service *api.Service) ([]int, error) { - ports := []int{} +func getPortsForLB(service *api.Service) ([]*api.ServicePort, error) { + ports := []*api.ServicePort{} for i := range service.Spec.Ports { // TODO: Support UDP. Remove the check from the API validation package once // it's supported. @@ -463,21 +463,58 @@ func getTCPPorts(service *api.Service) ([]int, error) { if sp.Protocol != api.ProtocolTCP { return nil, fmt.Errorf("external load balancers for non TCP services are not currently supported.") } - ports = append(ports, sp.Port) + ports = append(ports, sp) } return ports, nil } -func portsEqual(x, y *api.Service) bool { - xPorts, err := getTCPPorts(x) +func portsEqualForLB(x, y *api.Service) bool { + xPorts, err := getPortsForLB(x) if err != nil { return false } - yPorts, err := getTCPPorts(y) + yPorts, err := getPortsForLB(y) if err != nil { return false } - return intSlicesEqual(xPorts, yPorts) + return portSlicesEqualForLB(xPorts, yPorts) +} + +func portSlicesEqualForLB(x, y []*api.ServicePort) bool { + if len(x) != len(y) { + return false + } + + for i := range x { + if !portEqualForLB(x[i], y[i]) { + return false + } + } + return true +} + +func portEqualForLB(x, y *api.ServicePort) bool { + // TODO: Should we check name? (In theory, an LB could expose it) + if x.Name != y.Name { + return false + } + + if x.Protocol != y.Protocol { + return false + } + + if x.Port != y.Port { + return false + } + + if x.NodePort != y.NodePort { + return false + } + + // We don't check TargetPort; that is not relevant for load balancing + // TODO: Should we blank it out? Or just check it anyway? + + return true } func intSlicesEqual(x, y []int) bool { diff --git a/pkg/cloudprovider/servicecontroller/servicecontroller_test.go b/pkg/cloudprovider/servicecontroller/servicecontroller_test.go index d2e75bd45a8..d39112219f4 100644 --- a/pkg/cloudprovider/servicecontroller/servicecontroller_test.go +++ b/pkg/cloudprovider/servicecontroller/servicecontroller_test.go @@ -113,7 +113,7 @@ func TestCreateExternalLoadBalancer(t *testing.T) { t.Errorf("expected one load balancer to be created, got %v", cloud.Balancers) } else if cloud.Balancers[0].Name != controller.loadBalancerName(item.service) || cloud.Balancers[0].Region != region || - cloud.Balancers[0].Ports[0] != item.service.Spec.Ports[0].Port { + cloud.Balancers[0].Ports[0].Port != item.service.Spec.Ports[0].Port { t.Errorf("created load balancer has incorrect parameters: %v", cloud.Balancers[0]) } actionFound := false