diff --git a/api/swagger-spec/v1beta3.json b/api/swagger-spec/v1beta3.json index 78c5281d78a..d1da89a6a60 100644 --- a/api/swagger-spec/v1beta3.json +++ b/api/swagger-spec/v1beta3.json @@ -5722,4 +5722,4 @@ } } } - } \ No newline at end of file + } diff --git a/cluster/addons/cluster-monitoring/v1beta3/grafana-service.yaml b/cluster/addons/cluster-monitoring/v1beta3/grafana-service.yaml index bdf4080d0b1..611a8ae816a 100644 --- a/cluster/addons/cluster-monitoring/v1beta3/grafana-service.yaml +++ b/cluster/addons/cluster-monitoring/v1beta3/grafana-service.yaml @@ -6,8 +6,9 @@ metadata: kubernetes.io/cluster-service: "true" name: monitoring-grafana spec: - targetPort: 80 - port: 80 + ports: + - port: 80 + targetPort: 80 selector: name: influxGrafana kubernetes.io/cluster-service: "true" diff --git a/cluster/addons/cluster-monitoring/v1beta3/heapster-service.yaml b/cluster/addons/cluster-monitoring/v1beta3/heapster-service.yaml index e10a4f8d3fe..06919b01c46 100644 --- a/cluster/addons/cluster-monitoring/v1beta3/heapster-service.yaml +++ b/cluster/addons/cluster-monitoring/v1beta3/heapster-service.yaml @@ -6,8 +6,9 @@ metadata: kubernetes.io/cluster-service: "true" name: monitoring-heapster spec: - targetPort: 8082 - port: 80 + ports: + - port: 80 + targetPort: 8082 selector: name: heapster kubernetes.io/cluster-service: "true" diff --git a/cluster/addons/cluster-monitoring/v1beta3/influxdb-service.yaml b/cluster/addons/cluster-monitoring/v1beta3/influxdb-service.yaml index bf3465dc77f..1babd85ec38 100644 --- a/cluster/addons/cluster-monitoring/v1beta3/influxdb-service.yaml +++ b/cluster/addons/cluster-monitoring/v1beta3/influxdb-service.yaml @@ -5,8 +5,9 @@ metadata: name: influxGrafana name: monitoring-influxdb spec: - targetPort: 8086 - port: 80 + ports: + - port: 80 + targetPort: 8086 selector: name: influxGrafana diff --git a/cluster/addons/cluster-monitoring/v1beta3/influxdb-ui-service.yaml b/cluster/addons/cluster-monitoring/v1beta3/influxdb-ui-service.yaml index 5073a30817c..3c4f9d4d116 100644 --- a/cluster/addons/cluster-monitoring/v1beta3/influxdb-ui-service.yaml +++ b/cluster/addons/cluster-monitoring/v1beta3/influxdb-ui-service.yaml @@ -5,8 +5,9 @@ metadata: name: influxGrafana name: monitoring-influxdb-ui spec: - targetPort: 8083 - port: 80 + ports: + - port: 80 + targetPort: 8083 selector: name: influxGrafana diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 99cca0773cc..c28c6a35eab 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -465,12 +465,14 @@ func runSelfLinkTestOnNamespace(c *client.Client, namespace string) { }, }, Spec: api.ServiceSpec{ - Port: 12345, // This is here because validation requires it. Selector: map[string]string{ "foo": "bar", }, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 12345, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } @@ -527,12 +529,14 @@ func runAtomicPutTest(c *client.Client) { }, }, Spec: api.ServiceSpec{ - Port: 12345, // This is here because validation requires it. Selector: map[string]string{ "foo": "bar", }, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 12345, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } @@ -606,12 +610,14 @@ func runPatchTest(c *client.Client) { }, }, Spec: api.ServiceSpec{ - Port: 12345, // This is here because validation requires it. Selector: map[string]string{ "foo": "bar", }, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 12345, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } @@ -747,8 +753,10 @@ func runServiceTest(client *client.Client) { Selector: map[string]string{ "name": "thisisalonglabel", }, - Port: 8080, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 8080, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } @@ -764,8 +772,10 @@ func runServiceTest(client *client.Client) { Selector: map[string]string{ "name": "thisisalonglabel", }, - Port: 8080, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 8080, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } @@ -784,8 +794,10 @@ func runServiceTest(client *client.Client) { Selector: map[string]string{ "name": "thisisalonglabel", }, - Port: 8080, - Protocol: "TCP", + Ports: []api.ServicePort{{ + Port: 8080, + Protocol: "TCP", + }}, SessionAffinity: "None", }, } diff --git a/contrib/for-tests/network-tester/service.json b/contrib/for-tests/network-tester/service.json index e220d5cad19..4ad82545007 100644 --- a/contrib/for-tests/network-tester/service.json +++ b/contrib/for-tests/network-tester/service.json @@ -8,11 +8,15 @@ } }, "spec": { - "port": 8080, - "protocol": "TCP", + "ports": [ + { + "port": 8080, + "protocol": "TCP", + "targetPort": 8080 + } + ], "selector": { "name": "nettest" - }, - "targetPort": 8080, + } } } diff --git a/examples/cassandra/v1beta3/cassandra-service.yaml b/examples/cassandra/v1beta3/cassandra-service.yaml index 08a8f744dae..1811db7753f 100644 --- a/examples/cassandra/v1beta3/cassandra-service.yaml +++ b/examples/cassandra/v1beta3/cassandra-service.yaml @@ -5,7 +5,8 @@ metadata: name: cassandra name: cassandra spec: - targetPort: 9042 - port: 9042 + ports: + - port: 9042 + targetPort: 9042 selector: name: cassandra diff --git a/examples/guestbook-go/v1beta3/guestbook-service.json b/examples/guestbook-go/v1beta3/guestbook-service.json index c7973217e81..0f515b64f76 100644 --- a/examples/guestbook-go/v1beta3/guestbook-service.json +++ b/examples/guestbook-go/v1beta3/guestbook-service.json @@ -8,9 +8,13 @@ } }, "spec":{ - "port":3000, - "containerPort":"http-server", - "protocol":"TCP", + "ports": [ + { + "port":3000, + "targetPort":"http-server", + "protocol":"TCP" + } + ], "selector":{ "name":"guestbook" } diff --git a/examples/guestbook-go/v1beta3/redis-master-service.json b/examples/guestbook-go/v1beta3/redis-master-service.json index 2296638efba..5aed7d9ff84 100644 --- a/examples/guestbook-go/v1beta3/redis-master-service.json +++ b/examples/guestbook-go/v1beta3/redis-master-service.json @@ -9,9 +9,13 @@ } }, "spec":{ - "port":6379, - "containerPort":"redis-server", - "protocol":"TCP", + "ports": [ + { + "port":6379, + "targetPort":"redis-server", + "protocol":"TCP" + } + ], "selector":{ "name":"redis", "role":"master" diff --git a/examples/guestbook-go/v1beta3/redis-slave-service.json b/examples/guestbook-go/v1beta3/redis-slave-service.json index 04b4861aa3b..2eb1fb4ad04 100644 --- a/examples/guestbook-go/v1beta3/redis-slave-service.json +++ b/examples/guestbook-go/v1beta3/redis-slave-service.json @@ -9,9 +9,13 @@ } }, "spec":{ - "port":6379, - "containerPort":"redis-server", - "protocol":"TCP", + "ports": [ + { + "port":6379, + "targetPort":"redis-server", + "protocol":"TCP" + } + ], "selector":{ "name":"redis", "role":"slave" diff --git a/examples/guestbook/v1beta3/frontend-service.json b/examples/guestbook/v1beta3/frontend-service.json index 0921ba7fcb4..07e81f9942b 100644 --- a/examples/guestbook/v1beta3/frontend-service.json +++ b/examples/guestbook/v1beta3/frontend-service.json @@ -8,9 +8,13 @@ } }, "spec":{ - "port":80, - "containerPort":80, - "protocol":"TCP", + "ports": [ + { + "port":80, + "targetPort":80, + "protocol":"TCP" + } + ], "selector":{ "name":"frontend" } diff --git a/examples/guestbook/v1beta3/redis-master-service.json b/examples/guestbook/v1beta3/redis-master-service.json index c22669b62bf..101d9ea965c 100644 --- a/examples/guestbook/v1beta3/redis-master-service.json +++ b/examples/guestbook/v1beta3/redis-master-service.json @@ -8,9 +8,13 @@ } }, "spec":{ - "port":6379, - "containerPort":6379, - "protocol":"TCP", + "ports": [ + { + "port":6379, + "targetPort":6379, + "protocol":"TCP" + } + ], "selector":{ "name":"redis-master" } diff --git a/examples/guestbook/v1beta3/redis-slave-service.json b/examples/guestbook/v1beta3/redis-slave-service.json index ad8ddf92d61..2b866b6f94a 100644 --- a/examples/guestbook/v1beta3/redis-slave-service.json +++ b/examples/guestbook/v1beta3/redis-slave-service.json @@ -8,9 +8,13 @@ } }, "spec":{ - "port":6379, - "containerPort":6379, - "protocol":"TCP", + "ports": [ + { + "port":6379, + "targetPort":6379, + "protocol":"TCP" + } + ], "selector":{ "name":"redis-slave" } diff --git a/examples/hazelcast/v1beta3/hazelcast-service.yaml b/examples/hazelcast/v1beta3/hazelcast-service.yaml index 186eaad570f..1ea5a121209 100644 --- a/examples/hazelcast/v1beta3/hazelcast-service.yaml +++ b/examples/hazelcast/v1beta3/hazelcast-service.yaml @@ -5,7 +5,8 @@ metadata: name: hazelcast name: hazelcast spec: - targetPort: 5701 - port: 5701 + ports: + - port: 5701 + targetPort: 5701 selector: name: hazelcast diff --git a/examples/mysql-wordpress-pd/v1beta3/mysql-service.yaml b/examples/mysql-wordpress-pd/v1beta3/mysql-service.yaml index 38fb46a9dee..9848475d4ab 100644 --- a/examples/mysql-wordpress-pd/v1beta3/mysql-service.yaml +++ b/examples/mysql-wordpress-pd/v1beta3/mysql-service.yaml @@ -5,8 +5,9 @@ metadata: name: mysql name: mysql spec: - targetPort: 3306 - port: 3306 + ports: + - port: 3306 + targetPort: 3306 selector: name: mysql diff --git a/examples/mysql-wordpress-pd/v1beta3/wordpress-service.yaml b/examples/mysql-wordpress-pd/v1beta3/wordpress-service.yaml index 8069b06a2bd..a2aa333faf8 100644 --- a/examples/mysql-wordpress-pd/v1beta3/wordpress-service.yaml +++ b/examples/mysql-wordpress-pd/v1beta3/wordpress-service.yaml @@ -5,8 +5,9 @@ metadata: name: wpfrontend name: wpfrontend spec: - targetPort: 80 - port: 80 + ports: + - port: 80 + targetPort: 80 selector: name: wpfrontend diff --git a/examples/redis/v1beta3/redis-sentinel-service.yaml b/examples/redis/v1beta3/redis-sentinel-service.yaml index 2c8bcaa46d7..7078d182f3f 100644 --- a/examples/redis/v1beta3/redis-sentinel-service.yaml +++ b/examples/redis/v1beta3/redis-sentinel-service.yaml @@ -6,7 +6,8 @@ metadata: role: service name: redis-sentinel spec: - targetPort: 26379 - port: 26379 + ports: + - port: 26379 + targetPort: 26379 selector: redis-sentinel: "true" diff --git a/examples/rethinkdb/v1beta3/admin-service.yaml b/examples/rethinkdb/v1beta3/admin-service.yaml index 91fc19452ac..03e00840ffb 100644 --- a/examples/rethinkdb/v1beta3/admin-service.yaml +++ b/examples/rethinkdb/v1beta3/admin-service.yaml @@ -6,8 +6,9 @@ metadata: name: rethinkdb-admin namespace: rethinkdb spec: - targetPort: 8080 - port: 8080 + ports: + - port: 8080 + targetPort: 8080 selector: db: rethinkdb role: admin diff --git a/examples/rethinkdb/v1beta3/driver-service.yaml b/examples/rethinkdb/v1beta3/driver-service.yaml index 5b5163684f4..824afac8790 100644 --- a/examples/rethinkdb/v1beta3/driver-service.yaml +++ b/examples/rethinkdb/v1beta3/driver-service.yaml @@ -6,7 +6,8 @@ metadata: name: rethinkdb-driver namespace: rethinkdb spec: - targetPort: 28015 - port: 28015 + ports: + - port: 28015 + targetPort: 28015 selector: db: rethinkdb diff --git a/examples/walkthrough/v1beta3/service.yaml b/examples/walkthrough/v1beta3/service.yaml index a6a9c187d6d..58a459e5116 100644 --- a/examples/walkthrough/v1beta3/service.yaml +++ b/examples/walkthrough/v1beta3/service.yaml @@ -3,16 +3,14 @@ kind: Service metadata: name: nginx-example spec: - # the container on each pod to connect to, can be a name - # (e.g. 'www') or a number (e.g. 80) - targetPort: 80 - # the port that this service should serve on - port: 8000 - protocol: TCP + ports: + - port: 8000 # the port that this service should serve on + # the container on each pod to connect to, can be a name + # (e.g. 'www') or a number (e.g. 80) + targetPort: 80 + protocol: TCP # just like the selector in the replication controller, # but this time it identifies the set of pods to load balance # traffic to. selector: name: nginx - - diff --git a/pkg/api/endpoints/util.go b/pkg/api/endpoints/util.go index 6ca43a78d01..93c7362830f 100644 --- a/pkg/api/endpoints/util.go +++ b/pkg/api/endpoints/util.go @@ -97,12 +97,27 @@ func (set addressSet) Insert(addr *api.EndpointAddress) { func hashAddresses(addrs addressSet) string { // Flatten the list of addresses into a string so it can be used as a - // map key. + // map key. Unfortunately, DeepHashObject is implemented in terms of + // spew, and spew does not handle non-primitive mapo keys well. So + // first we collapse it into a slice, sort the slice, then hash that. + slice := []*api.EndpointAddress{} + for k := range addrs { + slice = append(slice, k) + } + sort.Sort(addrPtrsByIP(slice)) hasher := md5.New() - util.DeepHashObject(hasher, addrs) + util.DeepHashObject(hasher, slice) return hex.EncodeToString(hasher.Sum(nil)[0:]) } +type addrPtrsByIP []*api.EndpointAddress + +func (sl addrPtrsByIP) Len() int { return len(sl) } +func (sl addrPtrsByIP) Swap(i, j int) { sl[i], sl[j] = sl[j], sl[i] } +func (sl addrPtrsByIP) Less(i, j int) bool { + return bytes.Compare([]byte(sl[i].IP), []byte(sl[j].IP)) < 0 +} + // SortSubsets sorts an array of EndpointSubset objects in place. For ease of // use it returns the input slice. func SortSubsets(subsets []api.EndpointSubset) []api.EndpointSubset { diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 85a2d27eaff..7c7870d22f3 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -216,11 +216,18 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer { }, func(ss *api.ServiceSpec, c fuzz.Continue) { c.FuzzNoCustom(ss) // fuzz self without calling this function again - switch ss.TargetPort.Kind { - case util.IntstrInt: - ss.TargetPort.IntVal = 1 + ss.TargetPort.IntVal%65535 // non-zero - case util.IntstrString: - ss.TargetPort.StrVal = "x" + ss.TargetPort.StrVal // non-empty + if len(ss.Ports) == 0 { + // There must be at least 1 port. + ss.Ports = append(ss.Ports, api.ServicePort{}) + c.Fuzz(&ss.Ports[0]) + } + for i := range ss.Ports { + switch ss.Ports[i].TargetPort.Kind { + case util.IntstrInt: + ss.Ports[i].TargetPort.IntVal = 1 + ss.Ports[i].TargetPort.IntVal%65535 // non-zero + case util.IntstrString: + ss.Ports[i].TargetPort.StrVal = "x" + ss.Ports[i].TargetPort.StrVal // non-empty + } } }, ) diff --git a/pkg/api/types.go b/pkg/api/types.go index febcc710e9c..048cc684231 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -863,12 +863,8 @@ type ServiceStatus struct{} // ServiceSpec describes the attributes that a user creates on a service type ServiceSpec struct { - // Port is the TCP or UDP port that will be made available to each pod for connecting to the pods - // proxied by this service. - Port int `json:"port"` - - // Required: Supports "TCP" and "UDP". - Protocol Protocol `json:"protocol,omitempty"` + // Required: The list of ports that are exposed by this service. + Ports []ServicePort `json:"ports"` // This service will route traffic to pods having labels matching this selector. If empty or not present, // the service is assumed to have endpoints set by an external process and Kubernetes will not modify @@ -891,17 +887,32 @@ type ServiceSpec struct { // For hostnames, the user will use a CNAME record (instead of using an A record with the IP) PublicIPs []string `json:"publicIPs,omitempty"` - // TargetPort is the name or number of the port on the container to direct traffic to. - // This is useful if the containers the service points to have multiple open ports. - // Optional: If unspecified, the first port on the container will be used. - // As of v1beta3 this field will become required in the internal API, - // and the versioned APIs must provide a default value. - TargetPort util.IntOrString `json:"targetPort,omitempty"` - // Required: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity AffinityType `json:"sessionAffinity,omitempty"` } +type ServicePort struct { + // Optional if only one ServicePort is defined on this service: The + // name of this port within the service. This must be a DNS_LABEL. + // All ports within a ServiceSpec must have unique names. This maps to + // the 'Name' field in EndpointPort objects. + Name string `json:"name"` + + // The IP protocol for this port. Supports "TCP" and "UDP". + Protocol Protocol `json:"protocol"` + + // The port that will be exposed on the service. + Port int `json:"port"` + + // Optional: The target port on pods selected by this service. If this + // is a string, it will be looked up as a named port in the target + // Pod's container ports. If this is not specified, the first port on + // the destination pod will be used. This behavior is deprecated. As + // of v1beta3 the default value is the sames as the Port field (an + // identity map). + TargetPort util.IntOrString `json:"targetPort"` +} + // Service is a named abstraction of software service (for example, mysql) consisting of local port // (for example 3306) that the proxy listens on, and the selector that determines which pods // will answer requests sent through the proxy. diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index f59f0a04df5..ac2f3d43d88 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -703,14 +703,28 @@ func init() { return err } - out.Port = in.Spec.Port - out.Protocol = Protocol(in.Spec.Protocol) + // Produce legacy fields. + if len(in.Spec.Ports) > 0 { + out.PortName = in.Spec.Ports[0].Name + out.Port = in.Spec.Ports[0].Port + out.Protocol = Protocol(in.Spec.Ports[0].Protocol) + out.ContainerPort = in.Spec.Ports[0].TargetPort + } + // Copy modern fields. + for i := range in.Spec.Ports { + out.Ports = append(out.Ports, ServicePort{ + Name: in.Spec.Ports[i].Name, + Port: in.Spec.Ports[i].Port, + Protocol: Protocol(in.Spec.Ports[i].Protocol), + ContainerPort: in.Spec.Ports[i].TargetPort, + }) + } + if err := s.Convert(&in.Spec.Selector, &out.Selector, 0); err != nil { return err } out.CreateExternalLoadBalancer = in.Spec.CreateExternalLoadBalancer out.PublicIPs = in.Spec.PublicIPs - out.ContainerPort = in.Spec.TargetPort out.PortalIP = in.Spec.PortalIP if err := s.Convert(&in.Spec.SessionAffinity, &out.SessionAffinity, 0); err != nil { return err @@ -729,14 +743,31 @@ func init() { return err } - out.Spec.Port = in.Port - out.Spec.Protocol = newer.Protocol(in.Protocol) + if len(in.Ports) == 0 && in.Port != 0 { + // Use legacy fields to produce modern fields. + out.Spec.Ports = append(out.Spec.Ports, newer.ServicePort{ + Name: in.PortName, + Port: in.Port, + Protocol: newer.Protocol(in.Protocol), + TargetPort: in.ContainerPort, + }) + } else { + // Use modern fields, ignore legacy. + for i := range in.Ports { + out.Spec.Ports = append(out.Spec.Ports, newer.ServicePort{ + Name: in.Ports[i].Name, + Port: in.Ports[i].Port, + Protocol: newer.Protocol(in.Ports[i].Protocol), + TargetPort: in.Ports[i].ContainerPort, + }) + } + } + if err := s.Convert(&in.Selector, &out.Spec.Selector, 0); err != nil { return err } out.Spec.CreateExternalLoadBalancer = in.CreateExternalLoadBalancer out.Spec.PublicIPs = in.PublicIPs - out.Spec.TargetPort = in.ContainerPort out.Spec.PortalIP = in.PortalIP if err := s.Convert(&in.SessionAffinity, &out.Spec.SessionAffinity, 0); err != nil { return err diff --git a/pkg/api/v1beta1/conversion_test.go b/pkg/api/v1beta1/conversion_test.go index 548d7b365be..f8953895207 100644 --- a/pkg/api/v1beta1/conversion_test.go +++ b/pkg/api/v1beta1/conversion_test.go @@ -284,6 +284,191 @@ func TestServiceEmptySelector(t *testing.T) { } } +func TestServicePorts(t *testing.T) { + testCases := []struct { + given current.Service + expected newer.Service + roundtrip current.Service + }{ + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "legacy-with-defaults", + }, + Port: 111, + Protocol: current.ProtocolTCP, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Port: 111, + Protocol: newer.ProtocolTCP, + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Port: 111, + Protocol: current.ProtocolTCP, + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "legacy-full", + }, + PortName: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolTCP, + TargetPort: util.NewIntOrStringFromString("p"), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "both", + }, + PortName: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + Ports: []current.ServicePort{{ + Name: "q", + Port: 222, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "q", + Port: 222, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "q", + Port: 222, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "one", + }, + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "two", + }, + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromInt(76), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: newer.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(76), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromInt(76), + }}, + }, + }, + } + + for i, tc := range testCases { + // Convert versioned -> internal. + got := newer.Service{} + if err := Convert(&tc.given, &got); err != nil { + t.Errorf("[Case: %d] Unexpected error: %v", i, err) + continue + } + if !reflect.DeepEqual(got.Spec.Ports, tc.expected.Spec.Ports) { + t.Errorf("[Case: %d] Expected %v, got %v", i, tc.expected.Spec.Ports, got.Spec.Ports) + } + + // Convert internal -> versioned. + got2 := current.Service{} + if err := Convert(&got, &got2); err != nil { + t.Errorf("[Case: %d] Unexpected error: %v", i, err) + continue + } + if !reflect.DeepEqual(got2.Ports, tc.roundtrip.Ports) { + t.Errorf("[Case: %d] Expected %v, got %v", i, tc.roundtrip.Ports, got2.Ports) + } + } +} + func TestPullPolicyConversion(t *testing.T) { table := []struct { versioned current.PullPolicy @@ -527,7 +712,7 @@ func TestEndpointsConversion(t *testing.T) { continue } if got2.Protocol != tc.given.Protocol || !newer.Semantic.DeepEqual(got2.Endpoints, tc.given.Endpoints) { - t.Errorf("[Case: %d] Expected %#v, got %#v", i, tc.given.Endpoints, got2.Endpoints) + t.Errorf("[Case: %d] Expected %s %#v, got %s %#v", i, tc.given.Protocol, tc.given.Endpoints, got2.Protocol, got2.Endpoints) } } } diff --git a/pkg/api/v1beta1/defaults.go b/pkg/api/v1beta1/defaults.go index 629668d4e78..413dd98d6a0 100644 --- a/pkg/api/v1beta1/defaults.go +++ b/pkg/api/v1beta1/defaults.go @@ -68,6 +68,14 @@ func init() { obj.SessionAffinity = AffinityTypeNone } }, + func(obj *ServicePort) { + if obj.Protocol == "" { + obj.Protocol = ProtocolTCP + } + if obj.ContainerPort == util.NewIntOrStringFromInt(0) || obj.ContainerPort == util.NewIntOrStringFromString("") { + obj.ContainerPort = util.NewIntOrStringFromInt(obj.Port) + } + }, func(obj *PodSpec) { if obj.DNSPolicy == "" { obj.DNSPolicy = DNSClusterFirst diff --git a/pkg/api/v1beta1/defaults_test.go b/pkg/api/v1beta1/defaults_test.go index 9dd881eae90..fd54c913cd1 100644 --- a/pkg/api/v1beta1/defaults_test.go +++ b/pkg/api/v1beta1/defaults_test.go @@ -22,6 +22,7 @@ import ( current "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { @@ -148,3 +149,35 @@ func TestSetDefaultContainerManifestHostNetwork(t *testing.T) { t.Errorf("Expected container port to be defaulted, was made %d instead of %d", hostPortNum, portNum) } } + +func TestSetDefaultServicePort(t *testing.T) { + // Unchanged if set. + in := ¤t.Service{Ports: []current.ServicePort{{Protocol: "UDP", Port: 9376, ContainerPort: util.NewIntOrStringFromInt(118)}}} + out := roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolUDP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolUDP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != in.Ports[0].ContainerPort { + t.Errorf("Expected port %d, got %d", in.Ports[0].ContainerPort, out.Ports[0].ContainerPort) + } + + // Defaulted. + in = ¤t.Service{Ports: []current.ServicePort{{Protocol: "", Port: 9376, ContainerPort: util.NewIntOrStringFromInt(0)}}} + out = roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != util.NewIntOrStringFromInt(in.Ports[0].Port) { + t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort) + } + + // Defaulted. + in = ¤t.Service{Ports: []current.ServicePort{{Protocol: "", Port: 9376, ContainerPort: util.NewIntOrStringFromString("")}}} + out = roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != util.NewIntOrStringFromInt(in.Ports[0].Port) { + t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort) + } +} diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 65a23f75969..5edf19a51a4 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -718,14 +718,21 @@ type Service struct { // Required. Port int `json:"port" description:"port exposed by the service"` + // Optional: The name of the first port. + PortName string `json:"portName,omitempty" description:"the name of the first port; optional"` // Optional: Defaults to "TCP". Protocol Protocol `json:"protocol,omitempty" description:"protocol for port; must be UDP or TCP; TCP if unspecified"` + // ContainerPort is the name or number of the port on the container to direct traffic to. + // This is useful if the containers the service points to have multiple open ports. + // Optional: If unspecified, the first port on the container will be used. + ContainerPort util.IntOrString `json:"containerPort,omitempty" description:"number or name of the port to access on the containers belonging to pods targeted by the service; defaults to the container's first open port"` // This service's labels. Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize services"` // This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected. Selector map[string]string `json:"selector" description:"label keys and values that must match in order to receive traffic for this service; if empty, all pods are selected, if not specified, endpoints must be manually specified"` + // An external load balancer should be set up via the cloud-provider CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"` @@ -733,11 +740,6 @@ type Service struct { // users to handle external traffic that arrives at a node. PublicIPs []string `json:"publicIPs,omitempty" description:"externally visible IPs (e.g. load balancers) that should be proxied to this service"` - // ContainerPort is the name or number of the port on the container to direct traffic to. - // This is useful if the containers the service points to have multiple open ports. - // Optional: If unspecified, the first port on the container will be used. - ContainerPort util.IntOrString `json:"containerPort,omitempty" description:"number or name of the port to access on the containers belonging to pods targeted by the service; defaults to the container's first open port"` - // PortalIP is usually assigned by the master. If specified by the user // we will try to respect it or else fail the request. This field can // not be changed by updates. @@ -750,6 +752,35 @@ type Service struct { // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity AffinityType `json:"sessionAffinity,omitempty" description:"enable client IP based session affinity; must be ClientIP or None; defaults to None"` + + // Optional: Ports to expose on the service. If this field is + // specified, the legacy fields (Port, PortName, Protocol, and + // ContainerPort) will be overwritten by the first member of this + // array. If this field is not specified, it will be populated from + // the legacy fields. + Ports []ServicePort `json:"ports" description:"ports to be exposed on the service"` +} + +type ServicePort struct { + // Required: The name of this port within the service. This must be a + // DNS_LABEL. All ports within a ServiceSpec must have unique names. + // This maps to the 'Name' field in EndpointPort objects. + Name string `json:"name" description:"the name of this port; optional if only one port is defined"` + + // Optional: The IP protocol for this port. Supports "TCP" and "UDP", + // default is TCP. + Protocol Protocol `json:"protocol" description:"the protocol used by this port; must be UDP or TCP; TCP if unspecified"` + + // Required: The port that will be exposed. + Port int `json:"port" description:"the port number that is exposed"` + + // Optional: The port number on the target pod to direct traffic to. + // This is useful if the containers the service points to have multiple + // open ports. If this is a string, it will be looked up as a named + // port in the target Pod's container ports. If unspecified, the value + // of Port is used (an identity map) - note this is a different default + // than Service.ContainerPort. + ContainerPort util.IntOrString `json:"containerPort" description:"the port to access on the containers belonging to pods targeted by the service; defaults to the service port"` } // EndpointObjectReference is a reference to an object exposing the endpoint diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index 3149a21a3b2..0606ee14ee2 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/pkg/api/v1beta2/conversion.go @@ -635,14 +635,28 @@ func init() { return err } - out.Port = in.Spec.Port - out.Protocol = Protocol(in.Spec.Protocol) + // Produce legacy fields. + if len(in.Spec.Ports) > 0 { + out.PortName = in.Spec.Ports[0].Name + out.Port = in.Spec.Ports[0].Port + out.Protocol = Protocol(in.Spec.Ports[0].Protocol) + out.ContainerPort = in.Spec.Ports[0].TargetPort + } + // Copy modern fields. + for i := range in.Spec.Ports { + out.Ports = append(out.Ports, ServicePort{ + Name: in.Spec.Ports[i].Name, + Port: in.Spec.Ports[i].Port, + Protocol: Protocol(in.Spec.Ports[i].Protocol), + ContainerPort: in.Spec.Ports[i].TargetPort, + }) + } + if err := s.Convert(&in.Spec.Selector, &out.Selector, 0); err != nil { return err } out.CreateExternalLoadBalancer = in.Spec.CreateExternalLoadBalancer out.PublicIPs = in.Spec.PublicIPs - out.ContainerPort = in.Spec.TargetPort out.PortalIP = in.Spec.PortalIP if err := s.Convert(&in.Spec.SessionAffinity, &out.SessionAffinity, 0); err != nil { return err @@ -661,14 +675,31 @@ func init() { return err } - out.Spec.Port = in.Port - out.Spec.Protocol = newer.Protocol(in.Protocol) + if len(in.Ports) == 0 && in.Port != 0 { + // Use legacy fields to produce modern fields. + out.Spec.Ports = append(out.Spec.Ports, newer.ServicePort{ + Name: in.PortName, + Port: in.Port, + Protocol: newer.Protocol(in.Protocol), + TargetPort: in.ContainerPort, + }) + } else { + // Use modern fields, ignore legacy. + for i := range in.Ports { + out.Spec.Ports = append(out.Spec.Ports, newer.ServicePort{ + Name: in.Ports[i].Name, + Port: in.Ports[i].Port, + Protocol: newer.Protocol(in.Ports[i].Protocol), + TargetPort: in.Ports[i].ContainerPort, + }) + } + } + if err := s.Convert(&in.Selector, &out.Spec.Selector, 0); err != nil { return err } out.Spec.CreateExternalLoadBalancer = in.CreateExternalLoadBalancer out.Spec.PublicIPs = in.PublicIPs - out.Spec.TargetPort = in.ContainerPort out.Spec.PortalIP = in.PortalIP if err := s.Convert(&in.SessionAffinity, &out.Spec.SessionAffinity, 0); err != nil { return err diff --git a/pkg/api/v1beta2/conversion_test.go b/pkg/api/v1beta2/conversion_test.go index 98228441b04..a30a955dcd5 100644 --- a/pkg/api/v1beta2/conversion_test.go +++ b/pkg/api/v1beta2/conversion_test.go @@ -18,6 +18,7 @@ package v1beta2_test import ( "encoding/json" + "reflect" "testing" newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -58,6 +59,191 @@ func TestServiceEmptySelector(t *testing.T) { } } +func TestServicePorts(t *testing.T) { + testCases := []struct { + given current.Service + expected newer.Service + roundtrip current.Service + }{ + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "legacy-with-defaults", + }, + Port: 111, + Protocol: current.ProtocolTCP, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Port: 111, + Protocol: newer.ProtocolTCP, + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Port: 111, + Protocol: current.ProtocolTCP, + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "legacy-full", + }, + PortName: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolTCP, + TargetPort: util.NewIntOrStringFromString("p"), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "both", + }, + PortName: "p", + Port: 111, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromString("p"), + Ports: []current.ServicePort{{ + Name: "q", + Port: 222, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "q", + Port: 222, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "q", + Port: 222, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "one", + }, + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }}, + }, + }, + { + given: current.Service{ + TypeMeta: current.TypeMeta{ + ID: "two", + }, + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromInt(76), + }}, + }, + expected: newer.Service{ + Spec: newer.ServiceSpec{Ports: []newer.ServicePort{{ + Name: "p", + Port: 111, + Protocol: newer.ProtocolUDP, + TargetPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: newer.ProtocolTCP, + TargetPort: util.NewIntOrStringFromInt(76), + }}}, + }, + roundtrip: current.Service{ + Ports: []current.ServicePort{{ + Name: "p", + Port: 111, + Protocol: current.ProtocolUDP, + ContainerPort: util.NewIntOrStringFromInt(93), + }, { + Name: "q", + Port: 222, + Protocol: current.ProtocolTCP, + ContainerPort: util.NewIntOrStringFromInt(76), + }}, + }, + }, + } + + for i, tc := range testCases { + // Convert versioned -> internal. + got := newer.Service{} + if err := newer.Scheme.Convert(&tc.given, &got); err != nil { + t.Errorf("[Case: %d] Unexpected error: %v", i, err) + continue + } + if !reflect.DeepEqual(got.Spec.Ports, tc.expected.Spec.Ports) { + t.Errorf("[Case: %d] Expected %v, got %v", i, tc.expected.Spec.Ports, got.Spec.Ports) + } + + // Convert internal -> versioned. + got2 := current.Service{} + if err := newer.Scheme.Convert(&got, &got2); err != nil { + t.Errorf("[Case: %d] Unexpected error: %v", i, err) + continue + } + if !reflect.DeepEqual(got2.Ports, tc.roundtrip.Ports) { + t.Errorf("[Case: %d] Expected %v, got %v", i, tc.roundtrip.Ports, got2.Ports) + } + } +} + func TestNodeConversion(t *testing.T) { version, kind, err := newer.Scheme.ObjectVersionAndKind(¤t.Minion{}) if err != nil { diff --git a/pkg/api/v1beta2/defaults.go b/pkg/api/v1beta2/defaults.go index 067ee6b0a05..859ba180cf9 100644 --- a/pkg/api/v1beta2/defaults.go +++ b/pkg/api/v1beta2/defaults.go @@ -69,6 +69,14 @@ func init() { obj.SessionAffinity = AffinityTypeNone } }, + func(obj *ServicePort) { + if obj.Protocol == "" { + obj.Protocol = ProtocolTCP + } + if obj.ContainerPort == util.NewIntOrStringFromInt(0) || obj.ContainerPort == util.NewIntOrStringFromString("") { + obj.ContainerPort = util.NewIntOrStringFromInt(obj.Port) + } + }, func(obj *PodSpec) { if obj.DNSPolicy == "" { obj.DNSPolicy = DNSClusterFirst diff --git a/pkg/api/v1beta2/defaults_test.go b/pkg/api/v1beta2/defaults_test.go index 4051957750a..ccd3a8b04f1 100644 --- a/pkg/api/v1beta2/defaults_test.go +++ b/pkg/api/v1beta2/defaults_test.go @@ -22,6 +22,7 @@ import ( current "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { @@ -147,3 +148,35 @@ func TestSetDefaultContainerManifestHostNetwork(t *testing.T) { t.Errorf("Expected container port to be defaulted, was made %d instead of %d", hostPortNum, portNum) } } + +func TestSetDefaultServicePort(t *testing.T) { + // Unchanged if set. + in := ¤t.Service{Ports: []current.ServicePort{{Protocol: "UDP", Port: 9376, ContainerPort: util.NewIntOrStringFromInt(118)}}} + out := roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolUDP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolUDP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != in.Ports[0].ContainerPort { + t.Errorf("Expected port %d, got %d", in.Ports[0].ContainerPort, out.Ports[0].ContainerPort) + } + + // Defaulted. + in = ¤t.Service{Ports: []current.ServicePort{{Protocol: "", Port: 9376, ContainerPort: util.NewIntOrStringFromInt(0)}}} + out = roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != util.NewIntOrStringFromInt(in.Ports[0].Port) { + t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort) + } + + // Defaulted. + in = ¤t.Service{Ports: []current.ServicePort{{Protocol: "", Port: 9376, ContainerPort: util.NewIntOrStringFromString("")}}} + out = roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Ports[0].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Ports[0].Protocol) + } + if out.Ports[0].ContainerPort != util.NewIntOrStringFromInt(in.Ports[0].Port) { + t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort) + } +} diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 55088c4f804..bfc4466d220 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -719,14 +719,21 @@ type Service struct { // Required. Port int `json:"port" description:"port exposed by the service"` + // Optional: The name of the first port. + PortName string `json:"portName,omitempty" description:"the name of the first port; optional"` // Optional: Defaults to "TCP". Protocol Protocol `json:"protocol,omitempty" description:"protocol for port; must be UDP or TCP; TCP if unspecified"` + // ContainerPort is the name or number of the port on the container to direct traffic to. + // This is useful if the containers the service points to have multiple open ports. + // Optional: If unspecified, the first port on the container will be used. + ContainerPort util.IntOrString `json:"containerPort,omitempty" description:"number or name of the port to access on the containers belonging to pods targeted by the service; defaults to the container's first open port"` // This service's labels. Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize services"` // This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected. Selector map[string]string `json:"selector" description:"label keys and values that must match in order to receive traffic for this service; if empty, all pods are selected, if not specified, endpoints must be manually specified"` + // An external load balancer should be set up via the cloud-provider CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty" description:"set up a cloud-provider-specific load balancer on an external IP"` @@ -734,11 +741,6 @@ type Service struct { // users to handle external traffic that arrives at a node. PublicIPs []string `json:"publicIPs,omitempty" description:"externally visible IPs (e.g. load balancers) that should be proxied to this service"` - // ContainerPort is the name or number of the port on the container to direct traffic to. - // This is useful if the containers the service points to have multiple open ports. - // Optional: If unspecified, the first port on the container will be used. - ContainerPort util.IntOrString `json:"containerPort,omitempty" description:"number or name of the port to access on the containers belonging to pods targeted by the service; defaults to the container's first open port"` - // PortalIP is usually assigned by the master. If specified by the user // we will try to respect it or else fail the request. This field can // not be changed by updates. @@ -751,6 +753,35 @@ type Service struct { // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity AffinityType `json:"sessionAffinity,omitempty" description:"enable client IP based session affinity; must be ClientIP or None; defaults to None"` + + // Optional: Ports to expose on the service. If this field is + // specified, the legacy fields (Port, PortName, Protocol, and + // ContainerPort) will be overwritten by the first member of this + // array. If this field is not specified, it will be populated from + // the legacy fields. + Ports []ServicePort `json:"ports" description:"ports to be exposed on the service"` +} + +type ServicePort struct { + // Required: The name of this port within the service. This must be a + // DNS_LABEL. All ports within a ServiceSpec must have unique names. + // This maps to the 'Name' field in EndpointPort objects. + Name string `json:"name" description:"the name of this port; optional if only one port is defined"` + + // Optional: The IP protocol for this port. Supports "TCP" and "UDP", + // default is TCP. + Protocol Protocol `json:"protocol" description:"the protocol used by this port; must be UDP or TCP; TCP if unspecified"` + + // Required: The port that will be exposed. + Port int `json:"port" description:"the port number that is exposed"` + + // Optional: The port number on the target pod to direct traffic to. + // This is useful if the containers the service points to have multiple + // open ports. If this is a string, it will be looked up as a named + // port in the target Pod's container ports. If unspecified, the value + // of Port is used (an identity map) - note this is a different default + // than Service.ContainerPort. + ContainerPort util.IntOrString `json:"containerPort" description:"the port to access on the containers belonging to pods targeted by the service; defaults to the service port"` } // EndpointObjectReference is a reference to an object exposing the endpoint diff --git a/pkg/api/v1beta3/defaults.go b/pkg/api/v1beta3/defaults.go index 9e00a234303..558689d1001 100644 --- a/pkg/api/v1beta3/defaults.go +++ b/pkg/api/v1beta3/defaults.go @@ -53,13 +53,18 @@ func init() { } }, func(obj *Service) { - if obj.Spec.Protocol == "" { - obj.Spec.Protocol = ProtocolTCP - } if obj.Spec.SessionAffinity == "" { obj.Spec.SessionAffinity = AffinityTypeNone } }, + func(obj *ServicePort) { + if obj.Protocol == "" { + obj.Protocol = ProtocolTCP + } + if obj.TargetPort == util.NewIntOrStringFromInt(0) || obj.TargetPort == util.NewIntOrStringFromString("") { + obj.TargetPort = util.NewIntOrStringFromInt(obj.Port) + } + }, func(obj *PodSpec) { if obj.DNSPolicy == "" { obj.DNSPolicy = DNSClusterFirst @@ -97,9 +102,11 @@ func init() { obj.Path = "/" } }, - func(obj *ServiceSpec) { - if obj.TargetPort.Kind == util.IntstrInt && obj.TargetPort.IntVal == 0 || - obj.TargetPort.Kind == util.IntstrString && obj.TargetPort.StrVal == "" { + func(obj *ServicePort) { + if obj.Protocol == "" { + obj.Protocol = ProtocolTCP + } + if obj.TargetPort == util.NewIntOrStringFromInt(0) || obj.TargetPort == util.NewIntOrStringFromString("") { obj.TargetPort = util.NewIntOrStringFromInt(obj.Port) } }, diff --git a/pkg/api/v1beta3/defaults_test.go b/pkg/api/v1beta3/defaults_test.go index 6a7ea994c4a..76444c02f43 100644 --- a/pkg/api/v1beta3/defaults_test.go +++ b/pkg/api/v1beta3/defaults_test.go @@ -44,9 +44,6 @@ func TestSetDefaultService(t *testing.T) { svc := ¤t.Service{} obj2 := roundTrip(t, runtime.Object(svc)) svc2 := obj2.(*current.Service) - if svc2.Spec.Protocol != current.ProtocolTCP { - t.Errorf("Expected default protocol :%s, got: %s", current.ProtocolTCP, svc2.Spec.Protocol) - } if svc2.Spec.SessionAffinity != current.AffinityTypeNone { t.Errorf("Expected default sesseion affinity type:%s, got: %s", current.AffinityTypeNone, svc2.Spec.SessionAffinity) } @@ -85,18 +82,62 @@ func TestSetDefaulEndpointsProtocol(t *testing.T) { } func TestSetDefaulServiceTargetPort(t *testing.T) { - in := ¤t.Service{Spec: current.ServiceSpec{Port: 1234}} + in := ¤t.Service{Spec: current.ServiceSpec{Ports: []current.ServicePort{{Port: 1234}}}} obj := roundTrip(t, runtime.Object(in)) out := obj.(*current.Service) - if out.Spec.TargetPort.Kind != util.IntstrInt || out.Spec.TargetPort.IntVal != 1234 { - t.Errorf("Expected TargetPort to be defaulted, got %s", out.Spec.TargetPort) + if out.Spec.Ports[0].TargetPort != util.NewIntOrStringFromInt(1234) { + t.Errorf("Expected TargetPort to be defaulted, got %s", out.Spec.Ports[0].TargetPort) } - in = ¤t.Service{Spec: current.ServiceSpec{Port: 1234, TargetPort: util.NewIntOrStringFromInt(5678)}} + in = ¤t.Service{Spec: current.ServiceSpec{Ports: []current.ServicePort{{Port: 1234, TargetPort: util.NewIntOrStringFromInt(5678)}}}} obj = roundTrip(t, runtime.Object(in)) out = obj.(*current.Service) - if out.Spec.TargetPort.Kind != util.IntstrInt || out.Spec.TargetPort.IntVal != 5678 { - t.Errorf("Expected TargetPort to be unchanged, got %s", out.Spec.TargetPort) + if out.Spec.Ports[0].TargetPort != util.NewIntOrStringFromInt(5678) { + t.Errorf("Expected TargetPort to be unchanged, got %s", out.Spec.Ports[0].TargetPort) + } +} + +func TestSetDefaultServicePort(t *testing.T) { + // Unchanged if set. + in := ¤t.Service{Spec: current.ServiceSpec{ + Ports: []current.ServicePort{ + {Protocol: "UDP", Port: 9376, TargetPort: util.NewIntOrStringFromString("p")}, + {Protocol: "UDP", Port: 8675, TargetPort: util.NewIntOrStringFromInt(309)}, + }, + }} + out := roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Spec.Ports[0].Protocol != current.ProtocolUDP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolUDP, out.Spec.Ports[0].Protocol) + } + if out.Spec.Ports[0].TargetPort != util.NewIntOrStringFromString("p") { + t.Errorf("Expected port %d, got %s", in.Spec.Ports[0].Port, out.Spec.Ports[0].TargetPort) + } + if out.Spec.Ports[1].Protocol != current.ProtocolUDP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolUDP, out.Spec.Ports[1].Protocol) + } + if out.Spec.Ports[1].TargetPort != util.NewIntOrStringFromInt(309) { + t.Errorf("Expected port %d, got %s", in.Spec.Ports[1].Port, out.Spec.Ports[1].TargetPort) + } + + // Defaulted. + in = ¤t.Service{Spec: current.ServiceSpec{ + Ports: []current.ServicePort{ + {Protocol: "", Port: 9376, TargetPort: util.NewIntOrStringFromString("")}, + {Protocol: "", Port: 8675, TargetPort: util.NewIntOrStringFromInt(0)}, + }, + }} + out = roundTrip(t, runtime.Object(in)).(*current.Service) + if out.Spec.Ports[0].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Spec.Ports[0].Protocol) + } + if out.Spec.Ports[0].TargetPort != util.NewIntOrStringFromInt(in.Spec.Ports[0].Port) { + t.Errorf("Expected port %d, got %d", in.Spec.Ports[0].Port, out.Spec.Ports[0].TargetPort) + } + if out.Spec.Ports[1].Protocol != current.ProtocolTCP { + t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Spec.Ports[1].Protocol) + } + if out.Spec.Ports[1].TargetPort != util.NewIntOrStringFromInt(in.Spec.Ports[1].Port) { + t.Errorf("Expected port %d, got %d", in.Spec.Ports[1].Port, out.Spec.Ports[1].TargetPort) } } diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index d7837e3f77c..de7fdd661c8 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -853,12 +853,8 @@ type ServiceStatus struct{} // ServiceSpec describes the attributes that a user creates on a service type ServiceSpec struct { - // Port is the TCP or UDP port that will be made available to each pod for connecting to the pods - // proxied by this service. - Port int `json:"port" description:"port exposed by the service"` - - // Optional: Supports "TCP" and "UDP". Defaults to "TCP". - Protocol Protocol `json:"protocol,omitempty" description:"protocol for port; must be UDP or TCP; TCP if unspecified"` + // Required: The list of ports that are exposed by this service. + Ports []ServicePort `json:"ports" description:"ports exposed by the service"` // This service will route traffic to pods having labels matching this selector. If null, no endpoints will be automatically created. If empty, all pods will be selected. Selector map[string]string `json:"selector" description:"label keys and values that must match in order to receive traffic for this service; if empty, all pods are selected, if not specified, endpoints must be manually specified"` @@ -877,15 +873,31 @@ type ServiceSpec struct { // users to handle external traffic that arrives at a node. PublicIPs []string `json:"publicIPs,omitempty" description:"externally visible IPs (e.g. load balancers) that should be proxied to this service"` - // TargetPort is the name or number of the port on the container to direct traffic to. - // This is useful if the containers the service points to have multiple open ports. - // Optional: If unspecified, the service port is used (an identity map). - TargetPort util.IntOrString `json:"targetPort,omitempty" description:"number or name of the port to access on the containers belonging to pods targeted by the service; defaults to the container's first open port"` - // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity AffinityType `json:"sessionAffinity,omitempty" description:"enable client IP based session affinity; must be ClientIP or None; defaults to None"` } +type ServicePort struct { + // Optional if only one ServicePort is defined on this service: The + // name of this port within the service. This must be a DNS_LABEL. + // All ports within a ServiceSpec must have unique names. This maps to + // the 'Name' field in EndpointPort objects. + Name string `json:"name" description:"the name of this port; optional if only one port is defined"` + + // Optional: The IP protocol for this port. Supports "TCP" and "UDP", + // default is TCP. + Protocol Protocol `json:"protocol" description:"the protocol used by this port; must be UDP or TCP; TCP if unspecified"` + + // Required: The port that will be exposed by this service. + Port int `json:"port" description:"the port number that is exposed"` + + // Optional: The target port on pods selected by this service. + // If this is a string, it will be looked up as a named port in the + // target Pod's container ports. If this is not specified, the value + // of Port is used (an identity map). + TargetPort util.IntOrString `json:"targetPort" description:"the port to access on the pods targeted by the service; defaults to the service port"` +} + // Service is a named abstraction of software service (for example, mysql) consisting of local port // (for example 3306) that the proxy listens on, and the selector that determines which pods // will answer requests sent through the proxy. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 81c75de660e..ee9f4ff28c0 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -786,18 +786,12 @@ func ValidateService(service *api.Service) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} allErrs = append(allErrs, ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName).Prefix("metadata")...) - if !util.IsValidPortNum(service.Spec.Port) { - allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, portRangeErrorMsg)) + if len(service.Spec.Ports) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("spec.ports")) } - if len(service.Spec.Protocol) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("spec.protocol")) - } else if !supportedPortProtocols.Has(strings.ToUpper(string(service.Spec.Protocol))) { - allErrs = append(allErrs, errs.NewFieldNotSupported("spec.protocol", service.Spec.Protocol)) - } - if service.Spec.TargetPort.Kind == util.IntstrInt && service.Spec.TargetPort.IntVal != 0 && !util.IsValidPortNum(service.Spec.TargetPort.IntVal) { - allErrs = append(allErrs, errs.NewFieldInvalid("spec.containerPort", service.Spec.Port, portRangeErrorMsg)) - } else if service.Spec.TargetPort.Kind == util.IntstrString && len(service.Spec.TargetPort.StrVal) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("spec.containerPort")) + allPortNames := util.StringSet{} + for i := range service.Spec.Ports { + allErrs = append(allErrs, validateServicePort(&service.Spec.Ports[i], i, &allPortNames).PrefixIndex(i).Prefix("spec.ports")...) } if service.Spec.Selector != nil { @@ -827,6 +821,39 @@ func ValidateService(service *api.Service) errs.ValidationErrorList { return allErrs } +func validateServicePort(sp *api.ServicePort, index int, allNames *util.StringSet) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + + if len(sp.Name) == 0 { + // Allow empty names if they are the first port (mostly for compat). + if index != 0 { + allErrs = append(allErrs, errs.NewFieldRequired("name")) + } + } else if !util.IsDNS1123Label(sp.Name) { + allErrs = append(allErrs, errs.NewFieldInvalid("name", sp.Name, dns1123LabelErrorMsg)) + } else if allNames.Has(sp.Name) { + allErrs = append(allErrs, errs.NewFieldDuplicate("name", sp.Name)) + } + + if !util.IsValidPortNum(sp.Port) { + allErrs = append(allErrs, errs.NewFieldInvalid("port", sp.Port, portRangeErrorMsg)) + } + + if len(sp.Protocol) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("protocol")) + } else if !supportedPortProtocols.Has(strings.ToUpper(string(sp.Protocol))) { + allErrs = append(allErrs, errs.NewFieldNotSupported("protocol", sp.Protocol)) + } + + if sp.TargetPort != util.NewIntOrStringFromInt(0) && sp.TargetPort != util.NewIntOrStringFromString("") { + if sp.TargetPort.Kind == util.IntstrInt && !util.IsValidPortNum(sp.TargetPort.IntVal) { + allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portRangeErrorMsg)) + } + } + + return allErrs +} + // ValidateServiceUpdate tests if required fields in the service are set during an update func ValidateServiceUpdate(oldService, service *api.Service) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 615ff54162d..7e03baaf2b5 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1292,6 +1292,13 @@ func TestValidateService(t *testing.T) { }, numErrs: 1, }, + { + name: "nil selector", + makeSvc: func(s *api.Service) { + s.Spec.Selector = nil + }, + numErrs: 0, + }, { name: "invalid selector", makeSvc: func(s *api.Service) { @@ -1306,17 +1313,45 @@ func TestValidateService(t *testing.T) { }, numErrs: 1, }, + { + name: "missing ports", + makeSvc: func(s *api.Service) { + s.Spec.Ports = nil + }, + numErrs: 1, + }, + { + name: "empty port[0] name", + makeSvc: func(s *api.Service) { + s.Spec.Ports[0].Name = "" + }, + numErrs: 0, + }, + { + name: "empty port[1] name", + makeSvc: func(s *api.Service) { + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "", Protocol: "TCP", Port: 12345}) + }, + numErrs: 1, + }, + { + name: "invalid port name", + makeSvc: func(s *api.Service) { + s.Spec.Ports[0].Name = "INVALID" + }, + numErrs: 1, + }, { name: "missing protocol", makeSvc: func(s *api.Service) { - s.Spec.Protocol = "" + s.Spec.Ports[0].Protocol = "" }, numErrs: 1, }, { name: "invalid protocol", makeSvc: func(s *api.Service) { - s.Spec.Protocol = "INVALID" + s.Spec.Ports[0].Protocol = "INVALID" }, numErrs: 1, }, @@ -1330,28 +1365,21 @@ func TestValidateService(t *testing.T) { { name: "missing port", makeSvc: func(s *api.Service) { - s.Spec.Port = 0 + s.Spec.Ports[0].Port = 0 }, numErrs: 1, }, { name: "invalid port", makeSvc: func(s *api.Service) { - s.Spec.Port = 65536 + s.Spec.Ports[0].Port = 65536 }, numErrs: 1, }, { - name: "missing targetPort string", + name: "invalid TargetPort int", makeSvc: func(s *api.Service) { - s.Spec.TargetPort = util.NewIntOrStringFromString("") - }, - numErrs: 1, - }, - { - name: "invalid targetPort int", - makeSvc: func(s *api.Service) { - s.Spec.TargetPort = util.NewIntOrStringFromInt(65536) + s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(65536) }, numErrs: 1, }, @@ -1377,11 +1405,12 @@ func TestValidateService(t *testing.T) { numErrs: 0, }, { - name: "nil selector", + name: "dup port name", makeSvc: func(s *api.Service) { - s.Spec.Selector = nil + s.Spec.Ports[0].Name = "p" + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345}) }, - numErrs: 0, + numErrs: 1, }, { name: "valid 1", @@ -1393,15 +1422,15 @@ func TestValidateService(t *testing.T) { { name: "valid 2", makeSvc: func(s *api.Service) { - s.Spec.Protocol = "UDP" - s.Spec.TargetPort = util.NewIntOrStringFromInt(12345) + s.Spec.Ports[0].Protocol = "UDP" + s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(12345) }, numErrs: 0, }, { name: "valid 3", makeSvc: func(s *api.Service) { - s.Spec.TargetPort = util.NewIntOrStringFromString("http") + s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString("http") }, numErrs: 0, }, @@ -1416,6 +1445,7 @@ func TestValidateService(t *testing.T) { name: "valid portal ip - empty", makeSvc: func(s *api.Service) { s.Spec.PortalIP = "" + s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString("http") }, numErrs: 0, }, @@ -1432,8 +1462,7 @@ func TestValidateService(t *testing.T) { Spec: api.ServiceSpec{ Selector: map[string]string{"key": "val"}, SessionAffinity: "None", - Port: 8675, - Protocol: "TCP", + Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, }, } tc.makeSvc(&svc) diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index bafb75492f7..2385f005a17 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -694,7 +694,7 @@ func TestRequestDo(t *testing.T) { func TestDoRequestNewWay(t *testing.T) { reqBody := "request body" - expectedObj := &api.Service{Spec: api.ServiceSpec{Port: 12345}} + expectedObj := &api.Service{Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 12345}}}} expectedBody, _ := v1beta2.Codec.Encode(expectedObj) fakeHandler := util.FakeHandler{ StatusCode: 200, @@ -728,7 +728,7 @@ func TestDoRequestNewWay(t *testing.T) { func TestDoRequestNewWayReader(t *testing.T) { reqObj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} reqBodyExpected, _ := v1beta1.Codec.Encode(reqObj) - expectedObj := &api.Service{Spec: api.ServiceSpec{Port: 12345}} + expectedObj := &api.Service{Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 12345}}}} expectedBody, _ := v1beta1.Codec.Encode(expectedObj) fakeHandler := util.FakeHandler{ StatusCode: 200, @@ -764,7 +764,7 @@ func TestDoRequestNewWayReader(t *testing.T) { func TestDoRequestNewWayObj(t *testing.T) { reqObj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}} reqBodyExpected, _ := v1beta2.Codec.Encode(reqObj) - expectedObj := &api.Service{Spec: api.ServiceSpec{Port: 12345}} + expectedObj := &api.Service{Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 12345}}}} expectedBody, _ := v1beta2.Codec.Encode(expectedObj) fakeHandler := util.FakeHandler{ StatusCode: 200, @@ -814,7 +814,7 @@ func TestDoRequestNewWayFile(t *testing.T) { t.Errorf("unexpected error: %v", err) } - expectedObj := &api.Service{Spec: api.ServiceSpec{Port: 12345}} + expectedObj := &api.Service{Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 12345}}}} expectedBody, _ := v1beta1.Codec.Encode(expectedObj) fakeHandler := util.FakeHandler{ StatusCode: 200, @@ -855,7 +855,7 @@ func TestWasCreated(t *testing.T) { t.Errorf("unexpected error: %v", err) } - expectedObj := &api.Service{Spec: api.ServiceSpec{Port: 12345}} + expectedObj := &api.Service{Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 12345}}}} expectedBody, _ := v1beta1.Codec.Encode(expectedObj) fakeHandler := util.FakeHandler{ StatusCode: 201, diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 7aa1ff7ab11..c50d0627ebb 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -48,7 +48,7 @@ type TCPLoadBalancer interface { // TODO: Break this up into different interfaces (LB, etc) when we have more than one type of service TCPLoadBalancerExists(name, region string) (bool, error) // CreateTCPLoadBalancer creates a new tcp load balancer. Returns the IP address or hostname of the balancer - CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinityType api.AffinityType) (string, error) + CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.AffinityType) (string, error) // UpdateTCPLoadBalancer updates hosts under the specified load balancer. UpdateTCPLoadBalancer(name, region string, hosts []string) error // DeleteTCPLoadBalancer deletes a specified load balancer. diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index 7aa0b8fc24f..624c2d61d86 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/pkg/cloudprovider/fake/fake.go @@ -29,7 +29,7 @@ type FakeBalancer struct { Name string Region string ExternalIP net.IP - Port int + Ports []int Hosts []string } @@ -95,9 +95,9 @@ func (f *FakeCloud) TCPLoadBalancerExists(name, region string) (bool, error) { // 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, port int, hosts []string, affinityType api.AffinityType) (string, error) { +func (f *FakeCloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.AffinityType) (string, error) { f.addCall("create") - f.Balancers = append(f.Balancers, FakeBalancer{name, region, externalIP, port, hosts}) + f.Balancers = append(f.Balancers, FakeBalancer{name, region, externalIP, ports, hosts}) return f.ExternalIP.String(), f.Err } diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index a187bc2d333..9f0c040f83a 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -228,15 +228,29 @@ func translateAffinityType(affinityType api.AffinityType) GCEAffinityType { } // CreateTCPLoadBalancer is an implementation of TCPLoadBalancer.CreateTCPLoadBalancer. -func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinityType api.AffinityType) (string, error) { +func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinityType api.AffinityType) (string, error) { pool, err := gce.makeTargetPool(name, region, hosts, translateAffinityType(affinityType)) if err != nil { return "", err } + + if len(ports) == 0 { + return "", fmt.Errorf("no ports specified for GCE load balancer") + } + minPort := 65536 + maxPort := 0 + for i := range ports { + if ports[i] < minPort { + minPort = ports[i] + } + if ports[i] > maxPort { + maxPort = ports[i] + } + } req := &compute.ForwardingRule{ Name: name, IPProtocol: "TCP", - PortRange: strconv.Itoa(port), + PortRange: fmt.Sprintf("%d-%d", minPort, maxPort), Target: pool, } if len(externalIP) > 0 { diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 2d07106cc8d..2bfd98d4a0f 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -485,8 +485,8 @@ func (lb *LoadBalancer) TCPLoadBalancerExists(name, region string) (bool, error) // a list of regions (from config) and query/create loadbalancers in // each region. -func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinity api.AffinityType) (string, error) { - glog.V(4).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, port, hosts, affinity) +func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP net.IP, ports []int, hosts []string, affinity api.AffinityType) (string, error) { + glog.V(4).Infof("CreateTCPLoadBalancer(%v, %v, %v, %v, %v, %v)", name, region, externalIP, ports, hosts, affinity) var persistence *vips.SessionPersistence switch affinity { @@ -515,7 +515,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne _, err = members.Create(lb.network, members.CreateOpts{ PoolID: pool.ID, - ProtocolPort: port, + ProtocolPort: ports[0], //FIXME: need to handle multi-port Address: addr, }).Extract() if err != nil { @@ -550,7 +550,7 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne Description: fmt.Sprintf("Kubernetes external service %s", name), Address: externalIP.String(), Protocol: "TCP", - ProtocolPort: port, + ProtocolPort: ports[0], //FIXME: need to handle multi-port PoolID: pool.ID, Persistence: persistence, }).Extract() diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index fc5ea5d3101..10112153af9 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -65,7 +65,6 @@ func testData() (*api.PodList, *api.ServiceList, *api.ReplicationControllerList) { ObjectMeta: api.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"}, Spec: api.ServiceSpec{ - Protocol: "TCP", SessionAffinity: "None", }, }, diff --git a/pkg/kubectl/cmd/util/helpers_test.go b/pkg/kubectl/cmd/util/helpers_test.go index 142ad04d950..16730ba039f 100644 --- a/pkg/kubectl/cmd/util/helpers_test.go +++ b/pkg/kubectl/cmd/util/helpers_test.go @@ -135,15 +135,11 @@ func TestMerge(t *testing.T) { { kind: "Service", obj: &api.Service{ - Spec: api.ServiceSpec{ - Port: 10, - }, + Spec: api.ServiceSpec{}, }, fragment: `{ "apiVersion": "v1beta1", "port": 0 }`, expected: &api.Service{ Spec: api.ServiceSpec{ - Port: 0, - Protocol: "TCP", SessionAffinity: "None", }, }, @@ -160,7 +156,6 @@ func TestMerge(t *testing.T) { fragment: `{ "apiVersion": "v1beta1", "selector": { "version": "v2" } }`, expected: &api.Service{ Spec: api.ServiceSpec{ - Protocol: "TCP", SessionAffinity: "None", Selector: map[string]string{ "version": "v2", diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index 3f4b680b2cb..97907af03d4 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -333,7 +333,15 @@ func describeService(service *api.Service, endpoints *api.Endpoints, events *api list := strings.Join(service.Spec.PublicIPs, ", ") fmt.Fprintf(out, "Public IPs:\t%s\n", list) } - fmt.Fprintf(out, "Port:\t%d\n", service.Spec.Port) + for i := range service.Spec.Ports { + sp := &service.Spec.Ports[i] + + name := sp.Name + if name == "" { + name = "" + } + fmt.Fprintf(out, "Port:\t%s\t%d/%s\n", sp.Name, sp.Port, sp.Protocol) + } fmt.Fprintf(out, "Endpoints:\t%s\n", formatEndpoints(endpoints)) fmt.Fprintf(out, "Session Affinity:\t%s\n", service.Spec.SessionAffinity) if events != nil { diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index cb8414e2cf8..9acc9915dfd 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -228,7 +228,7 @@ func (h *HumanReadablePrinter) validatePrintHandlerFunc(printFunc reflect.Value) var podColumns = []string{"POD", "IP", "CONTAINER(S)", "IMAGE(S)", "HOST", "LABELS", "STATUS", "CREATED"} var replicationControllerColumns = []string{"CONTROLLER", "CONTAINER(S)", "IMAGE(S)", "SELECTOR", "REPLICAS"} -var serviceColumns = []string{"NAME", "LABELS", "SELECTOR", "IP", "PORT"} +var serviceColumns = []string{"NAME", "LABELS", "SELECTOR", "IP", "PORT(S)"} var endpointColumns = []string{"NAME", "ENDPOINTS"} var nodeColumns = []string{"NAME", "LABELS", "STATUS"} var statusColumns = []string{"STATUS"} @@ -390,9 +390,18 @@ func printReplicationControllerList(list *api.ReplicationControllerList, w io.Wr } func printService(svc *api.Service, w io.Writer) error { - _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\n", svc.Name, formatLabels(svc.Labels), - formatLabels(svc.Spec.Selector), svc.Spec.PortalIP, svc.Spec.Port) - return err + if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d/%s\n", svc.Name, formatLabels(svc.Labels), + formatLabels(svc.Spec.Selector), svc.Spec.PortalIP, svc.Spec.Ports[0].Port, svc.Spec.Ports[0].Protocol); err != nil { + return err + } + for i := 1; i < len(svc.Spec.Ports); i++ { + // Lay out additional ports. + if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d/%s\n", "", "", "", "", svc.Spec.Ports[i].Port, svc.Spec.Ports[i].Protocol); err != nil { + return err + } + } + + return nil } func printServiceList(list *api.ServiceList, w io.Writer) error { diff --git a/pkg/kubectl/service.go b/pkg/kubectl/service.go index ab4aadaff39..b3a600a7cbb 100644 --- a/pkg/kubectl/service.go +++ b/pkg/kubectl/service.go @@ -71,9 +71,14 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro Labels: labels, }, Spec: api.ServiceSpec{ - Port: port, - Protocol: api.Protocol(params["protocol"]), Selector: selector, + Ports: []api.ServicePort{ + { + Name: "default", + Port: port, + Protocol: api.Protocol(params["protocol"]), + }, + }, }, } targetPort, found := params["target-port"] @@ -82,12 +87,12 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro } if found && len(targetPort) > 0 { if portNum, err := strconv.Atoi(targetPort); err != nil { - service.Spec.TargetPort = util.NewIntOrStringFromString(targetPort) + service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString(targetPort) } else { - service.Spec.TargetPort = util.NewIntOrStringFromInt(portNum) + service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(portNum) } } else { - service.Spec.TargetPort = util.NewIntOrStringFromInt(port) + service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(port) } if params["create-external-load-balancer"] == "true" { service.Spec.CreateExternalLoadBalancer = true diff --git a/pkg/kubectl/service_test.go b/pkg/kubectl/service_test.go index 2c60bb7826a..7d679bc7487 100644 --- a/pkg/kubectl/service_test.go +++ b/pkg/kubectl/service_test.go @@ -46,9 +46,14 @@ func TestGenerateService(t *testing.T) { "foo": "bar", "baz": "blah", }, - Port: 80, - Protocol: "TCP", - TargetPort: util.NewIntOrStringFromInt(1234), + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "TCP", + TargetPort: util.NewIntOrStringFromInt(1234), + }, + }, }, }, }, @@ -69,9 +74,14 @@ func TestGenerateService(t *testing.T) { "foo": "bar", "baz": "blah", }, - Port: 80, - Protocol: "UDP", - TargetPort: util.NewIntOrStringFromString("foobar"), + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "UDP", + TargetPort: util.NewIntOrStringFromString("foobar"), + }, + }, }, }, }, @@ -97,9 +107,14 @@ func TestGenerateService(t *testing.T) { "foo": "bar", "baz": "blah", }, - Port: 80, - Protocol: "TCP", - TargetPort: util.NewIntOrStringFromInt(1234), + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "TCP", + TargetPort: util.NewIntOrStringFromInt(1234), + }, + }, }, }, }, @@ -121,10 +136,15 @@ func TestGenerateService(t *testing.T) { "foo": "bar", "baz": "blah", }, - Port: 80, - Protocol: "UDP", - PublicIPs: []string{"1.2.3.4"}, - TargetPort: util.NewIntOrStringFromString("foobar"), + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "UDP", + TargetPort: util.NewIntOrStringFromString("foobar"), + }, + }, + PublicIPs: []string{"1.2.3.4"}, }, }, }, @@ -147,10 +167,15 @@ func TestGenerateService(t *testing.T) { "foo": "bar", "baz": "blah", }, - Port: 80, - Protocol: "UDP", + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "UDP", + TargetPort: util.NewIntOrStringFromString("foobar"), + }, + }, PublicIPs: []string{"1.2.3.4"}, - TargetPort: util.NewIntOrStringFromString("foobar"), CreateExternalLoadBalancer: true, }, }, diff --git a/pkg/kubelet/envvars/envvars.go b/pkg/kubelet/envvars/envvars.go index 4848d7673f4..f548a73d39e 100644 --- a/pkg/kubelet/envvars/envvars.go +++ b/pkg/kubelet/envvars/envvars.go @@ -29,19 +29,30 @@ import ( // provided as an argument. func FromServices(services *api.ServiceList) []api.EnvVar { var result []api.EnvVar - for _, service := range services.Items { + for i := range services.Items { + service := &services.Items[i] + // ignore services where PortalIP is "None" or empty // the services passed to this method should be pre-filtered // only services that have the portal IP set should be included here - if !api.IsServiceIPSet(&service) { + if !api.IsServiceIPSet(service) { continue } + // Host name := makeEnvVariableName(service.Name) + "_SERVICE_HOST" result = append(result, api.EnvVar{Name: name, Value: service.Spec.PortalIP}) - // Port + // First port - give it the backwards-compatible name name = makeEnvVariableName(service.Name) + "_SERVICE_PORT" - result = append(result, api.EnvVar{Name: name, Value: strconv.Itoa(service.Spec.Port)}) + result = append(result, api.EnvVar{Name: name, Value: strconv.Itoa(service.Spec.Ports[0].Port)}) + // All named ports (only the first may be unnamed, checked in validation) + for i := range service.Spec.Ports { + sp := &service.Spec.Ports[i] + if sp.Name != "" { + pn := name + "_" + makeEnvVariableName(sp.Name) + result = append(result, api.EnvVar{Name: pn, Value: strconv.Itoa(sp.Port)}) + } + } // Docker-compatible vars. result = append(result, makeLinkVariables(service)...) } @@ -56,33 +67,42 @@ func makeEnvVariableName(str string) string { return strings.ToUpper(strings.Replace(str, "-", "_", -1)) } -func makeLinkVariables(service api.Service) []api.EnvVar { +func makeLinkVariables(service *api.Service) []api.EnvVar { prefix := makeEnvVariableName(service.Name) - protocol := string(api.ProtocolTCP) - if service.Spec.Protocol != "" { - protocol = string(service.Spec.Protocol) - } - portPrefix := fmt.Sprintf("%s_PORT_%d_%s", prefix, service.Spec.Port, strings.ToUpper(protocol)) - return []api.EnvVar{ - { - Name: prefix + "_PORT", - Value: fmt.Sprintf("%s://%s:%d", strings.ToLower(protocol), service.Spec.PortalIP, service.Spec.Port), - }, - { - Name: portPrefix, - Value: fmt.Sprintf("%s://%s:%d", strings.ToLower(protocol), service.Spec.PortalIP, service.Spec.Port), - }, - { - Name: portPrefix + "_PROTO", - Value: strings.ToLower(protocol), - }, - { - Name: portPrefix + "_PORT", - Value: strconv.Itoa(service.Spec.Port), - }, - { - Name: portPrefix + "_ADDR", - Value: service.Spec.PortalIP, - }, + all := []api.EnvVar{} + for i := range service.Spec.Ports { + sp := &service.Spec.Ports[i] + + protocol := string(api.ProtocolTCP) + if sp.Protocol != "" { + protocol = string(sp.Protocol) + } + if i == 0 { + // Docker special-cases the first port. + all = append(all, api.EnvVar{ + Name: prefix + "_PORT", + Value: fmt.Sprintf("%s://%s:%d", strings.ToLower(protocol), service.Spec.PortalIP, sp.Port), + }) + } + portPrefix := fmt.Sprintf("%s_PORT_%d_%s", prefix, sp.Port, strings.ToUpper(protocol)) + all = append(all, []api.EnvVar{ + { + Name: portPrefix, + Value: fmt.Sprintf("%s://%s:%d", strings.ToLower(protocol), service.Spec.PortalIP, sp.Port), + }, + { + Name: portPrefix + "_PROTO", + Value: strings.ToLower(protocol), + }, + { + Name: portPrefix + "_PORT", + Value: strconv.Itoa(sp.Port), + }, + { + Name: portPrefix + "_ADDR", + Value: service.Spec.PortalIP, + }, + }...) } + return all } diff --git a/pkg/kubelet/envvars/envvars_test.go b/pkg/kubelet/envvars/envvars_test.go index bff6726c081..44328faf02c 100644 --- a/pkg/kubelet/envvars/envvars_test.go +++ b/pkg/kubelet/envvars/envvars_test.go @@ -30,46 +30,53 @@ func TestFromServices(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "foo-bar"}, Spec: api.ServiceSpec{ - Port: 8080, Selector: map[string]string{"bar": "baz"}, - Protocol: "TCP", PortalIP: "1.2.3.4", + Ports: []api.ServicePort{ + {Port: 8080, Protocol: "TCP"}, + }, }, }, { ObjectMeta: api.ObjectMeta{Name: "abc-123"}, Spec: api.ServiceSpec{ - Port: 8081, Selector: map[string]string{"bar": "baz"}, - Protocol: "UDP", PortalIP: "5.6.7.8", + Ports: []api.ServicePort{ + {Name: "u-d-p", Port: 8081, Protocol: "UDP"}, + {Name: "t-c-p", Port: 8081, Protocol: "TCP"}, + }, }, }, { ObjectMeta: api.ObjectMeta{Name: "q-u-u-x"}, Spec: api.ServiceSpec{ - Port: 8082, Selector: map[string]string{"bar": "baz"}, - Protocol: "TCP", PortalIP: "9.8.7.6", + Ports: []api.ServicePort{ + {Port: 8082, Protocol: "TCP"}, + {Name: "8083", Port: 8083, Protocol: "TCP"}, + }, }, }, { ObjectMeta: api.ObjectMeta{Name: "svrc-portalip-none"}, Spec: api.ServiceSpec{ - Port: 8082, Selector: map[string]string{"bar": "baz"}, - Protocol: "TCP", PortalIP: "None", + Ports: []api.ServicePort{ + {Port: 8082, Protocol: "TCP"}, + }, }, }, { ObjectMeta: api.ObjectMeta{Name: "svrc-portalip-empty"}, Spec: api.ServiceSpec{ - Port: 8082, Selector: map[string]string{"bar": "baz"}, - Protocol: "TCP", PortalIP: "", + Ports: []api.ServicePort{ + {Port: 8082, Protocol: "TCP"}, + }, }, }, }, @@ -85,18 +92,29 @@ func TestFromServices(t *testing.T) { {Name: "FOO_BAR_PORT_8080_TCP_ADDR", Value: "1.2.3.4"}, {Name: "ABC_123_SERVICE_HOST", Value: "5.6.7.8"}, {Name: "ABC_123_SERVICE_PORT", Value: "8081"}, + {Name: "ABC_123_SERVICE_PORT_U_D_P", Value: "8081"}, + {Name: "ABC_123_SERVICE_PORT_T_C_P", Value: "8081"}, {Name: "ABC_123_PORT", Value: "udp://5.6.7.8:8081"}, {Name: "ABC_123_PORT_8081_UDP", Value: "udp://5.6.7.8:8081"}, {Name: "ABC_123_PORT_8081_UDP_PROTO", Value: "udp"}, {Name: "ABC_123_PORT_8081_UDP_PORT", Value: "8081"}, {Name: "ABC_123_PORT_8081_UDP_ADDR", Value: "5.6.7.8"}, + {Name: "ABC_123_PORT_8081_TCP", Value: "tcp://5.6.7.8:8081"}, + {Name: "ABC_123_PORT_8081_TCP_PROTO", Value: "tcp"}, + {Name: "ABC_123_PORT_8081_TCP_PORT", Value: "8081"}, + {Name: "ABC_123_PORT_8081_TCP_ADDR", Value: "5.6.7.8"}, {Name: "Q_U_U_X_SERVICE_HOST", Value: "9.8.7.6"}, {Name: "Q_U_U_X_SERVICE_PORT", Value: "8082"}, + {Name: "Q_U_U_X_SERVICE_PORT_8083", Value: "8083"}, {Name: "Q_U_U_X_PORT", Value: "tcp://9.8.7.6:8082"}, {Name: "Q_U_U_X_PORT_8082_TCP", Value: "tcp://9.8.7.6:8082"}, {Name: "Q_U_U_X_PORT_8082_TCP_PROTO", Value: "tcp"}, {Name: "Q_U_U_X_PORT_8082_TCP_PORT", Value: "8082"}, {Name: "Q_U_U_X_PORT_8082_TCP_ADDR", Value: "9.8.7.6"}, + {Name: "Q_U_U_X_PORT_8083_TCP", Value: "tcp://9.8.7.6:8083"}, + {Name: "Q_U_U_X_PORT_8083_TCP_PROTO", Value: "tcp"}, + {Name: "Q_U_U_X_PORT_8083_TCP_PORT", Value: "8083"}, + {Name: "Q_U_U_X_PORT_8083_TCP_ADDR", Value: "9.8.7.6"}, } if len(vars) != len(expected) { t.Errorf("Expected %d env vars, got: %+v", len(expected), vars) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 4a71542a01e..2a6898d4158 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -1787,97 +1787,139 @@ func TestMakeEnvironmentVariables(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Port: 8081, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8081, + }}, PortalIP: "1.2.3.1", }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes-ro", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Port: 8082, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8082, + }}, PortalIP: "1.2.3.2", }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes-ro", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Port: 8082, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8082, + }}, PortalIP: "None", }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes-ro", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Port: 8082, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8082, + }}, PortalIP: "", }, }, { ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test1"}, Spec: api.ServiceSpec{ - Port: 8083, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8083, + }}, PortalIP: "1.2.3.3", }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: "test2"}, Spec: api.ServiceSpec{ - Port: 8084, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8084, + }}, PortalIP: "1.2.3.4", }, }, { ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test2"}, Spec: api.ServiceSpec{ - Port: 8085, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8085, + }}, PortalIP: "1.2.3.5", }, }, { ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test2"}, Spec: api.ServiceSpec{ - Port: 8085, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8085, + }}, PortalIP: "None", }, }, { ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test2"}, Spec: api.ServiceSpec{ - Port: 8085, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8085, + }}, }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: "kubernetes"}, Spec: api.ServiceSpec{ - Port: 8086, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8086, + }}, PortalIP: "1.2.3.6", }, }, { ObjectMeta: api.ObjectMeta{Name: "kubernetes-ro", Namespace: "kubernetes"}, Spec: api.ServiceSpec{ - Port: 8087, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8087, + }}, PortalIP: "1.2.3.7", }, }, { ObjectMeta: api.ObjectMeta{Name: "not-special", Namespace: "kubernetes"}, Spec: api.ServiceSpec{ - Port: 8088, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8088, + }}, PortalIP: "1.2.3.8", }, }, { ObjectMeta: api.ObjectMeta{Name: "not-special", Namespace: "kubernetes"}, Spec: api.ServiceSpec{ - Port: 8088, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8088, + }}, PortalIP: "None", }, }, { ObjectMeta: api.ObjectMeta{Name: "not-special", Namespace: "kubernetes"}, Spec: api.ServiceSpec{ - Port: 8088, + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8088, + }}, PortalIP: "", }, }, diff --git a/pkg/master/publish.go b/pkg/master/publish.go index c3bd3bb0c5c..88688358c83 100644 --- a/pkg/master/publish.go +++ b/pkg/master/publish.go @@ -114,11 +114,10 @@ func (m *Master) createMasterServiceIfNeeded(serviceName string, serviceIP net.I Labels: map[string]string{"provider": "kubernetes", "component": "apiserver"}, }, Spec: api.ServiceSpec{ - Port: servicePort, + Ports: []api.ServicePort{{Port: servicePort, Protocol: api.ProtocolTCP}}, // maintained by this code, not by the pod selector Selector: nil, PortalIP: serviceIP.String(), - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, }, } diff --git a/pkg/proxy/config/config_test.go b/pkg/proxy/config/config_test.go index 927b939c1f8..89dac22d5f3 100644 --- a/pkg/proxy/config/config_test.go +++ b/pkg/proxy/config/config_test.go @@ -136,7 +136,10 @@ func TestNewServiceAddedAndNotified(t *testing.T) { handler := NewServiceHandlerMock() handler.Wait(1) config.RegisterHandler(handler) - serviceUpdate := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, Spec: api.ServiceSpec{Port: 10}}) + serviceUpdate := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 10}}}, + }) channel <- serviceUpdate handler.ValidateServices(t, serviceUpdate.Services) @@ -147,24 +150,35 @@ func TestServiceAddedRemovedSetAndNotified(t *testing.T) { channel := config.Channel("one") handler := NewServiceHandlerMock() config.RegisterHandler(handler) - serviceUpdate := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, Spec: api.ServiceSpec{Port: 10}}) + serviceUpdate := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 10}}}, + }) handler.Wait(1) channel <- serviceUpdate handler.ValidateServices(t, serviceUpdate.Services) - serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, Spec: api.ServiceSpec{Port: 20}}) + serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 20}}}, + }) handler.Wait(1) channel <- serviceUpdate2 services := []api.Service{serviceUpdate2.Services[0], serviceUpdate.Services[0]} handler.ValidateServices(t, services) - serviceUpdate3 := CreateServiceUpdate(REMOVE, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}}) + serviceUpdate3 := CreateServiceUpdate(REMOVE, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, + }) handler.Wait(1) channel <- serviceUpdate3 services = []api.Service{serviceUpdate2.Services[0]} handler.ValidateServices(t, services) - serviceUpdate4 := CreateServiceUpdate(SET, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foobar"}, Spec: api.ServiceSpec{Port: 99}}) + serviceUpdate4 := CreateServiceUpdate(SET, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foobar"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 99}}}, + }) handler.Wait(1) channel <- serviceUpdate4 services = []api.Service{serviceUpdate4.Services[0]} @@ -180,8 +194,14 @@ func TestNewMultipleSourcesServicesAddedAndNotified(t *testing.T) { } handler := NewServiceHandlerMock() config.RegisterHandler(handler) - serviceUpdate1 := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, Spec: api.ServiceSpec{Port: 10}}) - serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, Spec: api.ServiceSpec{Port: 20}}) + serviceUpdate1 := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 10}}}, + }) + serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 20}}}, + }) handler.Wait(2) channelOne <- serviceUpdate1 channelTwo <- serviceUpdate2 @@ -197,8 +217,14 @@ func TestNewMultipleSourcesServicesMultipleHandlersAddedAndNotified(t *testing.T handler2 := NewServiceHandlerMock() config.RegisterHandler(handler) config.RegisterHandler(handler2) - serviceUpdate1 := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, Spec: api.ServiceSpec{Port: 10}}) - serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, Spec: api.ServiceSpec{Port: 20}}) + serviceUpdate1 := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "foo"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 10}}}, + }) + serviceUpdate2 := CreateServiceUpdate(ADD, api.Service{ + ObjectMeta: api.ObjectMeta{Namespace: "testnamespace", Name: "bar"}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Protocol: "TCP", Port: 20}}}, + }) handler.Wait(2) handler2.Wait(2) channelOne <- serviceUpdate1 diff --git a/pkg/proxy/loadbalancer.go b/pkg/proxy/loadbalancer.go index 5f46bd639c5..bef117621a4 100644 --- a/pkg/proxy/loadbalancer.go +++ b/pkg/proxy/loadbalancer.go @@ -17,6 +17,7 @@ limitations under the License. package proxy import ( + "fmt" "net" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -27,7 +28,18 @@ import ( type LoadBalancer interface { // NextEndpoint returns the endpoint to handle a request for the given // service-port and source address. - NextEndpoint(service types.NamespacedName, port string, srcAddr net.Addr) (string, error) - NewService(service types.NamespacedName, port string, sessionAffinityType api.AffinityType, stickyMaxAgeMinutes int) error - CleanupStaleStickySessions(service types.NamespacedName, port string) + NextEndpoint(service ServicePortName, srcAddr net.Addr) (string, error) + NewService(service ServicePortName, sessionAffinityType api.AffinityType, stickyMaxAgeMinutes int) error + CleanupStaleStickySessions(service ServicePortName) +} + +// ServicePortName carries a namespace + name + portname. This is the unique +// identfier for a load-balanced service. +type ServicePortName struct { + types.NamespacedName + Port string +} + +func (spn ServicePortName) String() string { + return fmt.Sprintf("%s:%s", spn.NamespacedName.String(), spn.Port) } diff --git a/pkg/proxy/proxier.go b/pkg/proxy/proxier.go index 7a280fd7c5d..e6acd9712df 100644 --- a/pkg/proxy/proxier.go +++ b/pkg/proxy/proxier.go @@ -35,14 +35,13 @@ import ( ) type serviceInfo struct { - portalIP net.IP - portalPort int - protocol api.Protocol - proxyPort int - socket proxySocket - timeout time.Duration - // TODO: make this an net.IP address - publicIP []string + portalIP net.IP + portalPort int + protocol api.Protocol + proxyPort int + socket proxySocket + timeout time.Duration + publicIPs []string // TODO: make this net.IP sessionAffinityType api.AffinityType stickyMaxAgeMinutes int } @@ -59,7 +58,7 @@ type proxySocket interface { // while sessions are active. Close() error // ProxyLoop proxies incoming connections for the specified service to the service endpoints. - ProxyLoop(service types.NamespacedName, info *serviceInfo, proxier *Proxier) + ProxyLoop(service ServicePortName, info *serviceInfo, proxier *Proxier) } // tcpProxySocket implements proxySocket. Close() is implemented by net.Listener. When Close() is called, @@ -68,10 +67,9 @@ type tcpProxySocket struct { net.Listener } -func tryConnect(service types.NamespacedName, srcAddr net.Addr, protocol string, proxier *Proxier) (out net.Conn, err error) { +func tryConnect(service ServicePortName, srcAddr net.Addr, protocol string, proxier *Proxier) (out net.Conn, err error) { for _, retryTimeout := range endpointDialTimeout { - // TODO: support multiple service ports - endpoint, err := proxier.loadBalancer.NextEndpoint(service, "", srcAddr) + endpoint, err := proxier.loadBalancer.NextEndpoint(service, srcAddr) if err != nil { glog.Errorf("Couldn't find an endpoint for %s: %v", service, err) return nil, err @@ -89,7 +87,7 @@ func tryConnect(service types.NamespacedName, srcAddr net.Addr, protocol string, return nil, fmt.Errorf("failed to connect to an endpoint.") } -func (tcp *tcpProxySocket) ProxyLoop(service types.NamespacedName, myInfo *serviceInfo, proxier *Proxier) { +func (tcp *tcpProxySocket) ProxyLoop(service ServicePortName, myInfo *serviceInfo, proxier *Proxier) { for { if info, exists := proxier.getServiceInfo(service); !exists || info != myInfo { // The service port was closed or replaced. @@ -164,7 +162,7 @@ func newClientCache() *clientCache { return &clientCache{clients: map[string]net.Conn{}} } -func (udp *udpProxySocket) ProxyLoop(service types.NamespacedName, myInfo *serviceInfo, proxier *Proxier) { +func (udp *udpProxySocket) ProxyLoop(service ServicePortName, myInfo *serviceInfo, proxier *Proxier) { activeClients := newClientCache() var buffer [4096]byte // 4KiB should be enough for most whole-packets for { @@ -209,7 +207,7 @@ func (udp *udpProxySocket) ProxyLoop(service types.NamespacedName, myInfo *servi } } -func (udp *udpProxySocket) getBackendConn(activeClients *clientCache, cliAddr net.Addr, proxier *Proxier, service types.NamespacedName, timeout time.Duration) (net.Conn, error) { +func (udp *udpProxySocket) getBackendConn(activeClients *clientCache, cliAddr net.Addr, proxier *Proxier, service ServicePortName, timeout time.Duration) (net.Conn, error) { activeClients.mu.Lock() defer activeClients.mu.Unlock() @@ -305,7 +303,7 @@ func newProxySocket(protocol api.Protocol, ip net.IP, port int) (proxySocket, er type Proxier struct { loadBalancer LoadBalancer mu sync.Mutex // protects serviceMap - serviceMap map[types.NamespacedName]*serviceInfo + serviceMap map[ServicePortName]*serviceInfo numProxyLoops int32 // use atomic ops to access this; mostly for testing listenIP net.IP iptables iptables.Interface @@ -347,7 +345,7 @@ func CreateProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables } return &Proxier{ loadBalancer: loadBalancer, - serviceMap: make(map[types.NamespacedName]*serviceInfo), + serviceMap: make(map[ServicePortName]*serviceInfo), listenIP: listenIP, iptables: iptables, hostIP: hostIP, @@ -387,35 +385,32 @@ func (proxier *Proxier) ensurePortals() { // clean up any stale sticky session records in the hash map. func (proxier *Proxier) cleanupStaleStickySessions() { - for name, info := range proxier.serviceMap { - if info.sessionAffinityType != api.AffinityTypeNone { - // TODO: support multiple service ports - proxier.loadBalancer.CleanupStaleStickySessions(name, "") - } + for name := range proxier.serviceMap { + proxier.loadBalancer.CleanupStaleStickySessions(name) } } // This assumes proxier.mu is not locked. -func (proxier *Proxier) stopProxy(service types.NamespacedName, info *serviceInfo) error { +func (proxier *Proxier) stopProxy(service ServicePortName, info *serviceInfo) error { proxier.mu.Lock() defer proxier.mu.Unlock() return proxier.stopProxyInternal(service, info) } // This assumes proxier.mu is locked. -func (proxier *Proxier) stopProxyInternal(service types.NamespacedName, info *serviceInfo) error { +func (proxier *Proxier) stopProxyInternal(service ServicePortName, info *serviceInfo) error { delete(proxier.serviceMap, service) return info.socket.Close() } -func (proxier *Proxier) getServiceInfo(service types.NamespacedName) (*serviceInfo, bool) { +func (proxier *Proxier) getServiceInfo(service ServicePortName) (*serviceInfo, bool) { proxier.mu.Lock() defer proxier.mu.Unlock() info, ok := proxier.serviceMap[service] return info, ok } -func (proxier *Proxier) setServiceInfo(service types.NamespacedName, info *serviceInfo) { +func (proxier *Proxier) setServiceInfo(service ServicePortName, info *serviceInfo) { proxier.mu.Lock() defer proxier.mu.Unlock() proxier.serviceMap[service] = info @@ -424,7 +419,7 @@ func (proxier *Proxier) setServiceInfo(service types.NamespacedName, info *servi // addServiceOnPort starts listening for a new service, returning the serviceInfo. // Pass proxyPort=0 to allocate a random port. The timeout only applies to UDP // connections, for now. -func (proxier *Proxier) addServiceOnPort(service types.NamespacedName, protocol api.Protocol, proxyPort int, timeout time.Duration) (*serviceInfo, error) { +func (proxier *Proxier) addServiceOnPort(service ServicePortName, protocol api.Protocol, proxyPort int, timeout time.Duration) (*serviceInfo, error) { sock, err := newProxySocket(protocol, proxier.listenIP, proxyPort) if err != nil { return nil, err @@ -444,13 +439,13 @@ func (proxier *Proxier) addServiceOnPort(service types.NamespacedName, protocol protocol: protocol, socket: sock, timeout: timeout, - sessionAffinityType: api.AffinityTypeNone, - stickyMaxAgeMinutes: 180, + sessionAffinityType: api.AffinityTypeNone, // default + stickyMaxAgeMinutes: 180, // TODO: paramaterize this in the API. } proxier.setServiceInfo(service, si) glog.V(1).Infof("Proxying for service %q on %s port %d", service, protocol, portNum) - go func(service types.NamespacedName, proxier *Proxier) { + go func(service ServicePortName, proxier *Proxier) { defer util.HandleCrash() atomic.AddInt32(&proxier.numProxyLoops, 1) sock.ProxyLoop(service, si, proxier) @@ -468,51 +463,57 @@ const udpIdleTimeout = 1 * time.Minute // shutdown if missing from the update set. func (proxier *Proxier) OnUpdate(services []api.Service) { glog.V(4).Infof("Received update notice: %+v", services) - activeServices := make(map[types.NamespacedName]bool) // use a map as a set - for _, service := range services { - // if PortalIP is "None" or empty, skip proxying - if !api.IsServiceIPSet(&service) { - continue - } - serviceName := types.NamespacedName{service.Namespace, service.Name} - activeServices[serviceName] = true - info, exists := proxier.getServiceInfo(serviceName) - serviceIP := net.ParseIP(service.Spec.PortalIP) - // TODO: check health of the socket? What if ProxyLoop exited? - if exists && info.portalPort == service.Spec.Port && info.portalIP.Equal(serviceIP) && ipsEqual(service.Spec.PublicIPs, info.publicIP) { - continue - } - if exists { - glog.V(4).Infof("Something changed for service %q: stopping it", serviceName.String()) - err := proxier.closePortal(serviceName, info) - if err != nil { - glog.Errorf("Failed to close portal for %q: %v", serviceName, err) - } - err = proxier.stopProxy(serviceName, info) - if err != nil { - glog.Errorf("Failed to stop service %q: %v", serviceName, err) - } - } - glog.V(1).Infof("Adding new service %q at %s:%d/%s", serviceName, serviceIP, service.Spec.Port, service.Spec.Protocol) - info, err := proxier.addServiceOnPort(serviceName, service.Spec.Protocol, 0, udpIdleTimeout) - if err != nil { - glog.Errorf("Failed to start proxy for %q: %v", serviceName, err) - continue - } - info.portalIP = serviceIP - info.portalPort = service.Spec.Port - info.publicIP = service.Spec.PublicIPs - info.sessionAffinityType = service.Spec.SessionAffinity - // TODO: paramaterize this in the types api file as an attribute of sticky session. For now it's hardcoded to 3 hours. - info.stickyMaxAgeMinutes = 180 - glog.V(4).Infof("info: %+v", info) + activeServices := make(map[ServicePortName]bool) // use a map as a set + for i := range services { + service := &services[i] - err = proxier.openPortal(serviceName, info) - if err != nil { - glog.Errorf("Failed to open portal for %q: %v", serviceName, err) + // if PortalIP is "None" or empty, skip proxying + if !api.IsServiceIPSet(service) { + glog.V(3).Infof("Skipping service %s due to portal IP = %q", types.NamespacedName{service.Namespace, service.Name}, service.Spec.PortalIP) + continue + } + + for i := range service.Spec.Ports { + servicePort := &service.Spec.Ports[i] + + serviceName := ServicePortName{types.NamespacedName{service.Namespace, service.Name}, servicePort.Name} + activeServices[serviceName] = true + serviceIP := net.ParseIP(service.Spec.PortalIP) + info, exists := proxier.getServiceInfo(serviceName) + // TODO: check health of the socket? What if ProxyLoop exited? + if exists && sameConfig(info, service, servicePort) { + // Nothing changed. + continue + } + if exists { + glog.V(4).Infof("Something changed for service %q: stopping it", serviceName) + err := proxier.closePortal(serviceName, info) + if err != nil { + glog.Errorf("Failed to close portal for %q: %v", serviceName, err) + } + err = proxier.stopProxy(serviceName, info) + if err != nil { + glog.Errorf("Failed to stop service %q: %v", serviceName, err) + } + } + glog.V(1).Infof("Adding new service %q at %s:%d/%s", serviceName, serviceIP, servicePort.Port, servicePort.Protocol) + info, err := proxier.addServiceOnPort(serviceName, servicePort.Protocol, 0, udpIdleTimeout) + if err != nil { + glog.Errorf("Failed to start proxy for %q: %v", serviceName, err) + continue + } + info.portalIP = serviceIP + info.portalPort = servicePort.Port + info.publicIPs = service.Spec.PublicIPs + info.sessionAffinityType = service.Spec.SessionAffinity + glog.V(4).Infof("info: %+v", info) + + err = proxier.openPortal(serviceName, info) + if err != nil { + glog.Errorf("Failed to open portal for %q: %v", serviceName, err) + } + proxier.loadBalancer.NewService(serviceName, info.sessionAffinityType, info.stickyMaxAgeMinutes) } - // TODO: support multiple service ports - proxier.loadBalancer.NewService(serviceName, "", info.sessionAffinityType, info.stickyMaxAgeMinutes) } proxier.mu.Lock() defer proxier.mu.Unlock() @@ -531,6 +532,22 @@ func (proxier *Proxier) OnUpdate(services []api.Service) { } } +func sameConfig(info *serviceInfo, service *api.Service, port *api.ServicePort) bool { + if info.protocol != port.Protocol || info.portalPort != port.Port { + return false + } + if !info.portalIP.Equal(net.ParseIP(service.Spec.PortalIP)) { + return false + } + if !ipsEqual(info.publicIPs, service.Spec.PublicIPs) { + return false + } + if info.sessionAffinityType != service.Spec.SessionAffinity { + return false + } + return true +} + func ipsEqual(lhs, rhs []string) bool { if len(lhs) != len(rhs) { return false @@ -543,12 +560,12 @@ func ipsEqual(lhs, rhs []string) bool { return true } -func (proxier *Proxier) openPortal(service types.NamespacedName, info *serviceInfo) error { +func (proxier *Proxier) openPortal(service ServicePortName, info *serviceInfo) error { err := proxier.openOnePortal(info.portalIP, info.portalPort, info.protocol, proxier.listenIP, info.proxyPort, service) if err != nil { return err } - for _, publicIP := range info.publicIP { + for _, publicIP := range info.publicIPs { err = proxier.openOnePortal(net.ParseIP(publicIP), info.portalPort, info.protocol, proxier.listenIP, info.proxyPort, service) if err != nil { return err @@ -557,7 +574,7 @@ func (proxier *Proxier) openPortal(service types.NamespacedName, info *serviceIn return nil } -func (proxier *Proxier) openOnePortal(portalIP net.IP, portalPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, name types.NamespacedName) error { +func (proxier *Proxier) openOnePortal(portalIP net.IP, portalPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, name ServicePortName) error { // Handle traffic from containers. args := proxier.iptablesContainerPortalArgs(portalIP, portalPort, protocol, proxyIP, proxyPort, name) existed, err := proxier.iptables.EnsureRule(iptables.TableNAT, iptablesContainerPortalChain, args...) @@ -582,10 +599,10 @@ func (proxier *Proxier) openOnePortal(portalIP net.IP, portalPort int, protocol return nil } -func (proxier *Proxier) closePortal(service types.NamespacedName, info *serviceInfo) error { +func (proxier *Proxier) closePortal(service ServicePortName, info *serviceInfo) error { // Collect errors and report them all at the end. el := proxier.closeOnePortal(info.portalIP, info.portalPort, info.protocol, proxier.listenIP, info.proxyPort, service) - for _, publicIP := range info.publicIP { + for _, publicIP := range info.publicIPs { el = append(el, proxier.closeOnePortal(net.ParseIP(publicIP), info.portalPort, info.protocol, proxier.listenIP, info.proxyPort, service)...) } if len(el) == 0 { @@ -596,7 +613,7 @@ func (proxier *Proxier) closePortal(service types.NamespacedName, info *serviceI return errors.NewAggregate(el) } -func (proxier *Proxier) closeOnePortal(portalIP net.IP, portalPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, name types.NamespacedName) []error { +func (proxier *Proxier) closeOnePortal(portalIP net.IP, portalPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, name ServicePortName) []error { el := []error{} // Handle traffic from containers. @@ -675,7 +692,7 @@ var zeroIPv6 = net.ParseIP("::0") var localhostIPv6 = net.ParseIP("::1") // Build a slice of iptables args that are common to from-container and from-host portal rules. -func iptablesCommonPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, service types.NamespacedName) []string { +func iptablesCommonPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, service ServicePortName) []string { // This list needs to include all fields as they are eventually spit out // by iptables-save. This is because some systems do not support the // 'iptables -C' arg, and so fall back on parsing iptables-save output. @@ -696,7 +713,7 @@ func iptablesCommonPortalArgs(destIP net.IP, destPort int, protocol api.Protocol } // Build a slice of iptables args for a from-container portal rule. -func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service types.NamespacedName) []string { +func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service ServicePortName) []string { args := iptablesCommonPortalArgs(destIP, destPort, protocol, service) // This is tricky. @@ -743,7 +760,7 @@ func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, destPort int, } // Build a slice of iptables args for a from-host portal rule. -func (proxier *Proxier) iptablesHostPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service types.NamespacedName) []string { +func (proxier *Proxier) iptablesHostPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service ServicePortName) []string { args := iptablesCommonPortalArgs(destIP, destPort, protocol, service) // This is tricky. diff --git a/pkg/proxy/proxier_test.go b/pkg/proxy/proxier_test.go index 168cba5a259..bd88f80ccd5 100644 --- a/pkg/proxy/proxier_test.go +++ b/pkg/proxy/proxier_test.go @@ -195,13 +195,13 @@ func waitForNumProxyLoops(t *testing.T, p *Proxier, want int32) { func TestTCPProxy(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -219,13 +219,13 @@ func TestTCPProxy(t *testing.T) { func TestUDPProxy(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: udpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: udpServerPort}}, }}, }, }) @@ -241,8 +241,88 @@ func TestUDPProxy(t *testing.T) { waitForNumProxyLoops(t, p, 1) } +func TestMultiPortProxy(t *testing.T) { + lb := NewLoadBalancerRR() + serviceP := ServicePortName{types.NamespacedName{"testnamespace", "echo-p"}, "p"} + serviceQ := ServicePortName{types.NamespacedName{"testnamespace", "echo-q"}, "q"} + lb.OnUpdate([]api.Endpoints{{ + ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, + Subsets: []api.EndpointSubset{{ + Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, + Ports: []api.EndpointPort{{Name: "p", Protocol: "TCP", Port: tcpServerPort}}, + }}, + }, { + ObjectMeta: api.ObjectMeta{Name: serviceQ.Name, Namespace: serviceQ.Namespace}, + Subsets: []api.EndpointSubset{{ + Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, + Ports: []api.EndpointPort{{Name: "q", Protocol: "UDP", Port: udpServerPort}}, + }}, + }}) + + p := CreateProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + waitForNumProxyLoops(t, p, 0) + + svcInfoP, err := p.addServiceOnPort(serviceP, "TCP", 0, time.Second) + if err != nil { + t.Fatalf("error adding new service: %#v", err) + } + testEchoTCP(t, "127.0.0.1", svcInfoP.proxyPort) + waitForNumProxyLoops(t, p, 1) + + svcInfoQ, err := p.addServiceOnPort(serviceQ, "UDP", 0, time.Second) + if err != nil { + t.Fatalf("error adding new service: %#v", err) + } + testEchoUDP(t, "127.0.0.1", svcInfoQ.proxyPort) + waitForNumProxyLoops(t, p, 2) +} + +func TestMultiPortOnUpdate(t *testing.T) { + lb := NewLoadBalancerRR() + serviceP := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} + serviceQ := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "q"} + serviceX := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "x"} + + p := CreateProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + waitForNumProxyLoops(t, p, 0) + + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: 80, + Protocol: "TCP", + }, { + Name: "q", + Port: 81, + Protocol: "UDP", + }}}, + }}) + waitForNumProxyLoops(t, p, 2) + svcInfo, exists := p.getServiceInfo(serviceP) + if !exists { + t.Fatalf("can't find serviceInfo for %s", serviceP) + } + if svcInfo.portalIP.String() != "1.2.3.4" || svcInfo.portalPort != 80 || svcInfo.protocol != "TCP" { + t.Errorf("unexpected serviceInfo for %s: %#v", serviceP, svcInfo) + } + + svcInfo, exists = p.getServiceInfo(serviceQ) + if !exists { + t.Fatalf("can't find serviceInfo for %s", serviceQ) + } + if svcInfo.portalIP.String() != "1.2.3.4" || svcInfo.portalPort != 81 || svcInfo.protocol != "UDP" { + t.Errorf("unexpected serviceInfo for %s: %#v", serviceQ, svcInfo) + } + + svcInfo, exists = p.getServiceInfo(serviceX) + if exists { + t.Fatalf("found unwanted serviceInfo for %s: %#v", serviceX, svcInfo) + } +} + // Helper: Stops the proxy for the named service. -func stopProxyByName(proxier *Proxier, service types.NamespacedName) error { +func stopProxyByName(proxier *Proxier, service ServicePortName) error { info, found := proxier.getServiceInfo(service) if !found { return fmt.Errorf("unknown service: %s", service) @@ -252,13 +332,13 @@ func stopProxyByName(proxier *Proxier, service types.NamespacedName) error { func TestTCPProxyStop(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Namespace: service.Namespace, Name: service.Name}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -287,13 +367,13 @@ func TestTCPProxyStop(t *testing.T) { func TestUDPProxyStop(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Namespace: service.Namespace, Name: service.Name}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: udpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: udpServerPort}}, }}, }, }) @@ -322,13 +402,13 @@ func TestUDPProxyStop(t *testing.T) { func TestTCPProxyUpdateDelete(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Namespace: service.Namespace, Name: service.Name}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -356,13 +436,13 @@ func TestTCPProxyUpdateDelete(t *testing.T) { func TestUDPProxyUpdateDelete(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Namespace: service.Namespace, Name: service.Name}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: udpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: udpServerPort}}, }}, }, }) @@ -390,13 +470,13 @@ func TestUDPProxyUpdateDelete(t *testing.T) { func TestTCPProxyUpdateDeleteUpdate(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -420,9 +500,15 @@ func TestTCPProxyUpdateDeleteUpdate(t *testing.T) { t.Fatalf(err.Error()) } waitForNumProxyLoops(t, p, 0) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP", PortalIP: "1.2.3.4"}, Status: api.ServiceStatus{}}, - }) + + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.proxyPort, + Protocol: "TCP", + }}}, + }}) svcInfo, exists := p.getServiceInfo(service) if !exists { t.Fatalf("can't find serviceInfo for %s", service) @@ -433,13 +519,13 @@ func TestTCPProxyUpdateDeleteUpdate(t *testing.T) { func TestUDPProxyUpdateDeleteUpdate(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: udpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: udpServerPort}}, }}, }, }) @@ -463,9 +549,15 @@ func TestUDPProxyUpdateDeleteUpdate(t *testing.T) { t.Fatalf(err.Error()) } waitForNumProxyLoops(t, p, 0) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "UDP", PortalIP: "1.2.3.4"}, Status: api.ServiceStatus{}}, - }) + + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.proxyPort, + Protocol: "UDP", + }}}, + }}) svcInfo, exists := p.getServiceInfo(service) if !exists { t.Fatalf("can't find serviceInfo") @@ -476,13 +568,13 @@ func TestUDPProxyUpdateDeleteUpdate(t *testing.T) { func TestTCPProxyUpdatePort(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -497,9 +589,14 @@ func TestTCPProxyUpdatePort(t *testing.T) { testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: 99, Protocol: "TCP", PortalIP: "1.2.3.4"}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: 99, + Protocol: "TCP", + }}}, + }}) // Wait for the socket to actually get free. if err := waitForClosedPortTCP(p, svcInfo.proxyPort); err != nil { t.Fatalf(err.Error()) @@ -516,13 +613,13 @@ func TestTCPProxyUpdatePort(t *testing.T) { func TestUDPProxyUpdatePort(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: udpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: udpServerPort}}, }}, }, }) @@ -536,9 +633,14 @@ func TestUDPProxyUpdatePort(t *testing.T) { } waitForNumProxyLoops(t, p, 1) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: 99, Protocol: "UDP", PortalIP: "1.2.3.4"}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: 99, + Protocol: "UDP", + }}}, + }}) // Wait for the socket to actually get free. if err := waitForClosedPortUDP(p, svcInfo.proxyPort); err != nil { t.Fatalf(err.Error()) @@ -553,13 +655,13 @@ func TestUDPProxyUpdatePort(t *testing.T) { func TestProxyUpdatePublicIPs(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -574,9 +676,18 @@ func TestProxyUpdatePublicIPs(t *testing.T) { testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.portalPort, Protocol: "TCP", PortalIP: svcInfo.portalIP.String(), PublicIPs: []string{"4.3.2.1"}}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{ + Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.portalPort, + Protocol: "TCP", + }}, + PortalIP: svcInfo.portalIP.String(), + PublicIPs: []string{"4.3.2.1"}, + }, + }}) // Wait for the socket to actually get free. if err := waitForClosedPortTCP(p, svcInfo.proxyPort); err != nil { t.Fatalf(err.Error()) @@ -593,13 +704,13 @@ func TestProxyUpdatePublicIPs(t *testing.T) { func TestProxyUpdatePortal(t *testing.T) { lb := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "echo") + service := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "p"} lb.OnUpdate([]api.Endpoints{ { ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{ Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}}, - Ports: []api.EndpointPort{{Port: tcpServerPort}}, + Ports: []api.EndpointPort{{Name: "p", Port: tcpServerPort}}, }}, }, }) @@ -614,33 +725,40 @@ func TestProxyUpdatePortal(t *testing.T) { testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP"}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "", Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.proxyPort, + Protocol: "TCP", + }}}, + }}) _, exists := p.getServiceInfo(service) - if exists { - t.Fatalf("service without portalIP should not be included in the proxy") - } - - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP", PortalIP: ""}, Status: api.ServiceStatus{}}, - }) - _, exists = p.getServiceInfo(service) if exists { t.Fatalf("service with empty portalIP should not be included in the proxy") } - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP", PortalIP: "None"}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "None", Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.proxyPort, + Protocol: "TCP", + }}}, + }}) _, exists = p.getServiceInfo(service) if exists { t.Fatalf("service with 'None' as portalIP should not be included in the proxy") } - p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP", PortalIP: "1.2.3.4"}, Status: api.ServiceStatus{}}, - }) + p.OnUpdate([]api.Service{{ + ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + Spec: api.ServiceSpec{PortalIP: "1.2.3.4", Ports: []api.ServicePort{{ + Name: "p", + Port: svcInfo.proxyPort, + Protocol: "TCP", + }}}, + }}) svcInfo, exists = p.getServiceInfo(service) if !exists { t.Fatalf("service with portalIP set not found in the proxy") diff --git a/pkg/proxy/roundrobin.go b/pkg/proxy/roundrobin.go index dfa7028a216..c1bd4ab8354 100644 --- a/pkg/proxy/roundrobin.go +++ b/pkg/proxy/roundrobin.go @@ -50,20 +50,10 @@ type affinityPolicy struct { ttlMinutes int } -// servicePort is the type that the balancer uses to key stored state. -type servicePort struct { - types.NamespacedName - port string -} - -func (sp servicePort) String() string { - return fmt.Sprintf("%s:%s", sp.NamespacedName, sp.port) -} - // LoadBalancerRR is a round-robin load balancer. type LoadBalancerRR struct { lock sync.RWMutex - services map[servicePort]*balancerState + services map[ServicePortName]*balancerState } // Ensure this implements LoadBalancer. @@ -86,13 +76,11 @@ func newAffinityPolicy(affinityType api.AffinityType, ttlMinutes int) *affinityP // NewLoadBalancerRR returns a new LoadBalancerRR. func NewLoadBalancerRR() *LoadBalancerRR { return &LoadBalancerRR{ - services: map[servicePort]*balancerState{}, + services: map[ServicePortName]*balancerState{}, } } -func (lb *LoadBalancerRR) NewService(service types.NamespacedName, port string, affinityType api.AffinityType, ttlMinutes int) error { - svcPort := servicePort{service, port} - +func (lb *LoadBalancerRR) NewService(svcPort ServicePortName, affinityType api.AffinityType, ttlMinutes int) error { lb.lock.Lock() defer lb.lock.Unlock() lb.newServiceInternal(svcPort, affinityType, ttlMinutes) @@ -100,7 +88,7 @@ func (lb *LoadBalancerRR) NewService(service types.NamespacedName, port string, } // This assumes that lb.lock is already held. -func (lb *LoadBalancerRR) newServiceInternal(svcPort servicePort, affinityType api.AffinityType, ttlMinutes int) *balancerState { +func (lb *LoadBalancerRR) newServiceInternal(svcPort ServicePortName, affinityType api.AffinityType, ttlMinutes int) *balancerState { if ttlMinutes == 0 { ttlMinutes = 180 //default to 3 hours if not specified. Should 0 be unlimeted instead???? } @@ -123,9 +111,7 @@ func isSessionAffinity(affinity *affinityPolicy) bool { // NextEndpoint returns a service endpoint. // The service endpoint is chosen using the round-robin algorithm. -func (lb *LoadBalancerRR) NextEndpoint(service types.NamespacedName, port string, srcAddr net.Addr) (string, error) { - svcPort := servicePort{service, port} - +func (lb *LoadBalancerRR) NextEndpoint(svcPort ServicePortName, srcAddr net.Addr) (string, error) { // Coarse locking is simple. We can get more fine-grained if/when we // can prove it matters. lb.lock.Lock() @@ -202,7 +188,7 @@ func flattenValidEndpoints(endpoints []hostPortPair) []string { } // Remove any session affinity records associated to a particular endpoint (for example when a pod goes down). -func removeSessionAffinityByEndpoint(state *balancerState, svcPort servicePort, endpoint string) { +func removeSessionAffinityByEndpoint(state *balancerState, svcPort ServicePortName, endpoint string) { for _, affinity := range state.affinity.affinityMap { if affinity.endpoint == endpoint { glog.V(4).Infof("Removing client: %s from affinityMap for service %q", affinity.endpoint, svcPort) @@ -214,7 +200,7 @@ func removeSessionAffinityByEndpoint(state *balancerState, svcPort servicePort, // Loop through the valid endpoints and then the endpoints associated with the Load Balancer. // Then remove any session affinity records that are not in both lists. // This assumes the lb.lock is held. -func (lb *LoadBalancerRR) updateAffinityMap(svcPort servicePort, newEndpoints []string) { +func (lb *LoadBalancerRR) updateAffinityMap(svcPort ServicePortName, newEndpoints []string) { allEndpoints := map[string]int{} for _, newEndpoint := range newEndpoints { allEndpoints[newEndpoint] = 1 @@ -238,7 +224,7 @@ func (lb *LoadBalancerRR) updateAffinityMap(svcPort servicePort, newEndpoints [] // Registered endpoints are updated if found in the update set or // unregistered if missing from the update set. func (lb *LoadBalancerRR) OnUpdate(allEndpoints []api.Endpoints) { - registeredEndpoints := make(map[servicePort]bool) + registeredEndpoints := make(map[ServicePortName]bool) lb.lock.Lock() defer lb.lock.Unlock() @@ -262,7 +248,7 @@ func (lb *LoadBalancerRR) OnUpdate(allEndpoints []api.Endpoints) { } for portname := range portsToEndpoints { - svcPort := servicePort{types.NamespacedName{svcEndpoints.Namespace, svcEndpoints.Name}, portname} + svcPort := ServicePortName{types.NamespacedName{svcEndpoints.Namespace, svcEndpoints.Name}, portname} state, exists := lb.services[svcPort] curEndpoints := []string{} if state != nil { @@ -305,15 +291,12 @@ func slicesEquiv(lhs, rhs []string) bool { return false } -func (lb *LoadBalancerRR) CleanupStaleStickySessions(service types.NamespacedName, port string) { - svcPort := servicePort{service, port} - +func (lb *LoadBalancerRR) CleanupStaleStickySessions(svcPort ServicePortName) { lb.lock.Lock() defer lb.lock.Unlock() state, exists := lb.services[svcPort] if !exists { - glog.Warning("CleanupStaleStickySessions called for non-existent balancer key %q", svcPort) return } for ip, affinity := range state.affinity.affinityMap { diff --git a/pkg/proxy/roundrobin_test.go b/pkg/proxy/roundrobin_test.go index 4a4e00aa462..a4bc1a3c699 100644 --- a/pkg/proxy/roundrobin_test.go +++ b/pkg/proxy/roundrobin_test.go @@ -67,8 +67,8 @@ func TestLoadBalanceFailsWithNoEndpoints(t *testing.T) { loadBalancer := NewLoadBalancerRR() var endpoints []api.Endpoints loadBalancer.OnUpdate(endpoints) - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "does-not-exist", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "does-not-exist"} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil { t.Errorf("Didn't fail with non-existent service") } @@ -77,20 +77,20 @@ func TestLoadBalanceFailsWithNoEndpoints(t *testing.T) { } } -func expectEndpoint(t *testing.T, loadBalancer *LoadBalancerRR, service types.NamespacedName, port string, expected string, netaddr net.Addr) { - endpoint, err := loadBalancer.NextEndpoint(service, port, netaddr) +func expectEndpoint(t *testing.T, loadBalancer *LoadBalancerRR, service ServicePortName, expected string, netaddr net.Addr) { + endpoint, err := loadBalancer.NextEndpoint(service, netaddr) if err != nil { - t.Errorf("Didn't find a service for %s:%s, expected %s, failed with: %v", service, port, expected, err) + t.Errorf("Didn't find a service for %s, expected %s, failed with: %v", service, expected, err) } if endpoint != expected { - t.Errorf("Didn't get expected endpoint for service %s:%s client %v, expected %s, got: %s", service, port, netaddr, expected, endpoint) + t.Errorf("Didn't get expected endpoint for service %s client %v, expected %s, got: %s", service, netaddr, expected, endpoint) } } func TestLoadBalanceWorksWithSingleEndpoint(t *testing.T) { loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "p", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "p"} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } @@ -103,10 +103,10 @@ func TestLoadBalanceWorksWithSingleEndpoint(t *testing.T) { }}, } loadBalancer.OnUpdate(endpoints) - expectEndpoint(t, loadBalancer, service, "p", "endpoint1:40", nil) - expectEndpoint(t, loadBalancer, service, "p", "endpoint1:40", nil) - expectEndpoint(t, loadBalancer, service, "p", "endpoint1:40", nil) - expectEndpoint(t, loadBalancer, service, "p", "endpoint1:40", nil) + expectEndpoint(t, loadBalancer, service, "endpoint1:40", nil) + expectEndpoint(t, loadBalancer, service, "endpoint1:40", nil) + expectEndpoint(t, loadBalancer, service, "endpoint1:40", nil) + expectEndpoint(t, loadBalancer, service, "endpoint1:40", nil) } func stringsInSlice(haystack []string, needles ...string) bool { @@ -127,8 +127,8 @@ func stringsInSlice(haystack []string, needles ...string) bool { func TestLoadBalanceWorksWithMultipleEndpoints(t *testing.T) { loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "p", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "p"} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } @@ -142,26 +142,27 @@ func TestLoadBalanceWorksWithMultipleEndpoints(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, "p"}].endpoints + shuffledEndpoints := loadBalancer.services[service].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint:1", "endpoint:2", "endpoint:3") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[2], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], nil) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], nil) } func TestLoadBalanceWorksWithMultipleEndpointsMultiplePorts(t *testing.T) { loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "p", nil) + serviceP := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "p"} + serviceQ := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "q"} + endpoint, err := loadBalancer.NextEndpoint(serviceP, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ - ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, Subsets: []api.EndpointSubset{ { Addresses: []api.EndpointAddress{{IP: "endpoint1"}, {IP: "endpoint2"}}, @@ -175,35 +176,36 @@ func TestLoadBalanceWorksWithMultipleEndpointsMultiplePorts(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, "p"}].endpoints + shuffledEndpoints := loadBalancer.services[serviceP].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint1:1", "endpoint2:1", "endpoint3:3") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[2], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[2], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) - shuffledEndpoints = loadBalancer.services[servicePort{service, "q"}].endpoints + shuffledEndpoints = loadBalancer.services[serviceQ].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint1:2", "endpoint2:2", "endpoint3:4") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[2], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[2], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) } func TestLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "p", nil) + serviceP := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "p"} + serviceQ := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "q"} + endpoint, err := loadBalancer.NextEndpoint(serviceP, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ - ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, Subsets: []api.EndpointSubset{ { Addresses: []api.EndpointAddress{{IP: "endpoint1"}}, @@ -221,28 +223,28 @@ func TestLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, "p"}].endpoints + shuffledEndpoints := loadBalancer.services[serviceP].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint1:1", "endpoint2:2", "endpoint3:3") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[2], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[2], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) - shuffledEndpoints = loadBalancer.services[servicePort{service, "q"}].endpoints + shuffledEndpoints = loadBalancer.services[serviceQ].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint1:10", "endpoint2:20", "endpoint3:30") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[2], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[2], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) // Then update the configuration with one fewer endpoints, make sure // we start in the beginning again endpoints[0] = api.Endpoints{ - ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, + ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, Subsets: []api.EndpointSubset{ { Addresses: []api.EndpointAddress{{IP: "endpoint4"}}, @@ -256,29 +258,29 @@ func TestLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints = loadBalancer.services[servicePort{service, "p"}].endpoints + shuffledEndpoints = loadBalancer.services[serviceP].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint4:4", "endpoint5:5") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "p", shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceP, shuffledEndpoints[1], nil) - shuffledEndpoints = loadBalancer.services[servicePort{service, "q"}].endpoints + shuffledEndpoints = loadBalancer.services[serviceQ].endpoints if !stringsInSlice(shuffledEndpoints, "endpoint4:40", "endpoint5:50") { t.Errorf("did not find expected endpoints: %v", shuffledEndpoints) } - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[1], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[0], nil) - expectEndpoint(t, loadBalancer, service, "q", shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[1], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[0], nil) + expectEndpoint(t, loadBalancer, serviceQ, shuffledEndpoints[1], nil) // Clear endpoints - endpoints[0] = api.Endpoints{ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: nil} + endpoints[0] = api.Endpoints{ObjectMeta: api.ObjectMeta{Name: serviceP.Name, Namespace: serviceP.Namespace}, Subsets: nil} loadBalancer.OnUpdate(endpoints) - endpoint, err = loadBalancer.NextEndpoint(service, "p", nil) + endpoint, err = loadBalancer.NextEndpoint(serviceP, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } @@ -286,15 +288,15 @@ func TestLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { func TestLoadBalanceWorksWithServiceRemoval(t *testing.T) { loadBalancer := NewLoadBalancerRR() - fooService := types.NewNamespacedNameOrDie("testnamespace", "foo") - barService := types.NewNamespacedNameOrDie("testnamespace", "bar") - endpoint, err := loadBalancer.NextEndpoint(fooService, "p", nil) + fooServiceP := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, "p"} + barServiceP := ServicePortName{types.NamespacedName{"testnamespace", "bar"}, "p"} + endpoint, err := loadBalancer.NextEndpoint(fooServiceP, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } endpoints := make([]api.Endpoints, 2) endpoints[0] = api.Endpoints{ - ObjectMeta: api.ObjectMeta{Name: fooService.Name, Namespace: fooService.Namespace}, + ObjectMeta: api.ObjectMeta{Name: fooServiceP.Name, Namespace: fooServiceP.Namespace}, Subsets: []api.EndpointSubset{ { Addresses: []api.EndpointAddress{{IP: "endpoint1"}, {IP: "endpoint2"}, {IP: "endpoint3"}}, @@ -303,7 +305,7 @@ func TestLoadBalanceWorksWithServiceRemoval(t *testing.T) { }, } endpoints[1] = api.Endpoints{ - ObjectMeta: api.ObjectMeta{Name: barService.Name, Namespace: barService.Namespace}, + ObjectMeta: api.ObjectMeta{Name: barServiceP.Name, Namespace: barServiceP.Namespace}, Subsets: []api.EndpointSubset{ { Addresses: []api.EndpointAddress{{IP: "endpoint4"}, {IP: "endpoint5"}, {IP: "endpoint6"}}, @@ -312,54 +314,54 @@ func TestLoadBalanceWorksWithServiceRemoval(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledFooEndpoints := loadBalancer.services[servicePort{fooService, "p"}].endpoints - expectEndpoint(t, loadBalancer, fooService, "p", shuffledFooEndpoints[0], nil) - expectEndpoint(t, loadBalancer, fooService, "p", shuffledFooEndpoints[1], nil) - expectEndpoint(t, loadBalancer, fooService, "p", shuffledFooEndpoints[2], nil) - expectEndpoint(t, loadBalancer, fooService, "p", shuffledFooEndpoints[0], nil) - expectEndpoint(t, loadBalancer, fooService, "p", shuffledFooEndpoints[1], nil) + shuffledFooEndpoints := loadBalancer.services[fooServiceP].endpoints + expectEndpoint(t, loadBalancer, fooServiceP, shuffledFooEndpoints[0], nil) + expectEndpoint(t, loadBalancer, fooServiceP, shuffledFooEndpoints[1], nil) + expectEndpoint(t, loadBalancer, fooServiceP, shuffledFooEndpoints[2], nil) + expectEndpoint(t, loadBalancer, fooServiceP, shuffledFooEndpoints[0], nil) + expectEndpoint(t, loadBalancer, fooServiceP, shuffledFooEndpoints[1], nil) - shuffledBarEndpoints := loadBalancer.services[servicePort{barService, "p"}].endpoints - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[0], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[1], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[2], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[0], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[1], nil) + shuffledBarEndpoints := loadBalancer.services[barServiceP].endpoints + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[0], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[1], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[2], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[0], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[1], nil) // Then update the configuration by removing foo loadBalancer.OnUpdate(endpoints[1:]) - endpoint, err = loadBalancer.NextEndpoint(fooService, "p", nil) + endpoint, err = loadBalancer.NextEndpoint(fooServiceP, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } // but bar is still there, and we continue RR from where we left off. - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[2], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[0], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[1], nil) - expectEndpoint(t, loadBalancer, barService, "p", shuffledBarEndpoints[2], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[2], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[0], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[1], nil) + expectEndpoint(t, loadBalancer, barServiceP, shuffledBarEndpoints[2], nil) } func TestStickyLoadBalanceWorksWithSingleEndpoint(t *testing.T) { client1 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0} client2 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2), Port: 0} loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(service, "", api.AffinityTypeClientIP, 0) + loadBalancer.NewService(service, api.AffinityTypeClientIP, 0) endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: []api.EndpointSubset{{Addresses: []api.EndpointAddress{{IP: "endpoint"}}, Ports: []api.EndpointPort{{Port: 1}}}}, } loadBalancer.OnUpdate(endpoints) - expectEndpoint(t, loadBalancer, service, "", "endpoint:1", client1) - expectEndpoint(t, loadBalancer, service, "", "endpoint:1", client1) - expectEndpoint(t, loadBalancer, service, "", "endpoint:1", client2) - expectEndpoint(t, loadBalancer, service, "", "endpoint:1", client2) + expectEndpoint(t, loadBalancer, service, "endpoint:1", client1) + expectEndpoint(t, loadBalancer, service, "endpoint:1", client1) + expectEndpoint(t, loadBalancer, service, "endpoint:1", client2) + expectEndpoint(t, loadBalancer, service, "endpoint:1", client2) } func TestStickyLoadBalanaceWorksWithMultipleEndpoints(t *testing.T) { @@ -367,13 +369,13 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpoints(t *testing.T) { client2 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2), Port: 0} client3 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 3), Port: 0} loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(service, "", api.AffinityTypeClientIP, 0) + loadBalancer.NewService(service, api.AffinityTypeClientIP, 0) endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, @@ -385,15 +387,15 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpoints(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) + shuffledEndpoints := loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) } func TestStickyLoadBalanaceWorksWithMultipleEndpointsStickyNone(t *testing.T) { @@ -401,13 +403,13 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsStickyNone(t *testing.T) { client2 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2), Port: 0} client3 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 3), Port: 0} loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(service, "", api.AffinityTypeNone, 0) + loadBalancer.NewService(service, api.AffinityTypeNone, 0) endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, @@ -420,15 +422,15 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsStickyNone(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client1) + shuffledEndpoints := loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client1) } func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { @@ -439,13 +441,13 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { client5 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 5), Port: 0} client6 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 6), Port: 0} loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(service, "", api.AffinityTypeClientIP, 0) + loadBalancer.NewService(service, api.AffinityTypeClientIP, 0) endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, @@ -457,14 +459,14 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) + shuffledEndpoints := loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) client1Endpoint := shuffledEndpoints[0] - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) client2Endpoint := shuffledEndpoints[1] - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client3) client3Endpoint := shuffledEndpoints[2] endpoints[0] = api.Endpoints{ @@ -477,7 +479,7 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints = loadBalancer.services[servicePort{service, ""}].endpoints + shuffledEndpoints = loadBalancer.services[service].endpoints if client1Endpoint == "endpoint:3" { client1Endpoint = shuffledEndpoints[0] } else if client2Endpoint == "endpoint:3" { @@ -485,9 +487,9 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { } else if client3Endpoint == "endpoint:3" { client3Endpoint = shuffledEndpoints[0] } - expectEndpoint(t, loadBalancer, service, "", client1Endpoint, client1) - expectEndpoint(t, loadBalancer, service, "", client2Endpoint, client2) - expectEndpoint(t, loadBalancer, service, "", client3Endpoint, client3) + expectEndpoint(t, loadBalancer, service, client1Endpoint, client1) + expectEndpoint(t, loadBalancer, service, client2Endpoint, client2) + expectEndpoint(t, loadBalancer, service, client3Endpoint, client3) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, @@ -499,13 +501,13 @@ func TestStickyLoadBalanaceWorksWithMultipleEndpointsRemoveOne(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints = loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", client1Endpoint, client1) - expectEndpoint(t, loadBalancer, service, "", client2Endpoint, client2) - expectEndpoint(t, loadBalancer, service, "", client3Endpoint, client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client4) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client5) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client6) + shuffledEndpoints = loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, client1Endpoint, client1) + expectEndpoint(t, loadBalancer, service, client2Endpoint, client2) + expectEndpoint(t, loadBalancer, service, client3Endpoint, client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client4) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client5) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client6) } func TestStickyLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { @@ -513,13 +515,13 @@ func TestStickyLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { client2 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2), Port: 0} client3 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 3), Port: 0} loadBalancer := NewLoadBalancerRR() - service := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(service, "", nil) + service := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(service, "", api.AffinityTypeClientIP, 0) + loadBalancer.NewService(service, api.AffinityTypeClientIP, 0) endpoints := make([]api.Endpoints, 1) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, @@ -531,14 +533,14 @@ func TestStickyLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints := loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[2], client3) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) + shuffledEndpoints := loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[2], client3) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) // Then update the configuration with one fewer endpoints, make sure // we start in the beginning again endpoints[0] = api.Endpoints{ @@ -551,19 +553,19 @@ func TestStickyLoadBalanceWorksWithMultipleEndpointsAndUpdates(t *testing.T) { }, } loadBalancer.OnUpdate(endpoints) - shuffledEndpoints = loadBalancer.services[servicePort{service, ""}].endpoints - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[0], client1) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) - expectEndpoint(t, loadBalancer, service, "", shuffledEndpoints[1], client2) + shuffledEndpoints = loadBalancer.services[service].endpoints + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[0], client1) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) + expectEndpoint(t, loadBalancer, service, shuffledEndpoints[1], client2) // Clear endpoints endpoints[0] = api.Endpoints{ObjectMeta: api.ObjectMeta{Name: service.Name, Namespace: service.Namespace}, Subsets: nil} loadBalancer.OnUpdate(endpoints) - endpoint, err = loadBalancer.NextEndpoint(service, "", nil) + endpoint, err = loadBalancer.NextEndpoint(service, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } @@ -574,12 +576,12 @@ func TestStickyLoadBalanceWorksWithServiceRemoval(t *testing.T) { client2 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 2), Port: 0} client3 := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 3), Port: 0} loadBalancer := NewLoadBalancerRR() - fooService := types.NewNamespacedNameOrDie("testnamespace", "foo") - endpoint, err := loadBalancer.NextEndpoint(fooService, "", nil) + fooService := ServicePortName{types.NamespacedName{"testnamespace", "foo"}, ""} + endpoint, err := loadBalancer.NextEndpoint(fooService, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } - loadBalancer.NewService(fooService, "", api.AffinityTypeClientIP, 0) + loadBalancer.NewService(fooService, api.AffinityTypeClientIP, 0) endpoints := make([]api.Endpoints, 2) endpoints[0] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: fooService.Name, Namespace: fooService.Namespace}, @@ -590,8 +592,8 @@ func TestStickyLoadBalanceWorksWithServiceRemoval(t *testing.T) { }, }, } - barService := types.NewNamespacedNameOrDie("testnamespace", "bar") - loadBalancer.NewService(barService, "", api.AffinityTypeClientIP, 0) + barService := ServicePortName{types.NamespacedName{"testnamespace", "bar"}, ""} + loadBalancer.NewService(barService, api.AffinityTypeClientIP, 0) endpoints[1] = api.Endpoints{ ObjectMeta: api.ObjectMeta{Name: barService.Name, Namespace: barService.Namespace}, Subsets: []api.EndpointSubset{ @@ -603,38 +605,38 @@ func TestStickyLoadBalanceWorksWithServiceRemoval(t *testing.T) { } loadBalancer.OnUpdate(endpoints) - shuffledFooEndpoints := loadBalancer.services[servicePort{fooService, ""}].endpoints - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[0], client1) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[1], client2) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[2], client3) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[0], client1) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[0], client1) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[1], client2) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[1], client2) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[2], client3) - expectEndpoint(t, loadBalancer, fooService, "", shuffledFooEndpoints[2], client3) + shuffledFooEndpoints := loadBalancer.services[fooService].endpoints + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[0], client1) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[1], client2) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[2], client3) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[0], client1) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[0], client1) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[1], client2) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[1], client2) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[2], client3) + expectEndpoint(t, loadBalancer, fooService, shuffledFooEndpoints[2], client3) - shuffledBarEndpoints := loadBalancer.services[servicePort{barService, ""}].endpoints - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[1], client2) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[1], client2) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[1], client2) + shuffledBarEndpoints := loadBalancer.services[barService].endpoints + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[1], client2) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[1], client2) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[1], client2) // Then update the configuration by removing foo loadBalancer.OnUpdate(endpoints[1:]) - endpoint, err = loadBalancer.NextEndpoint(fooService, "", nil) + endpoint, err = loadBalancer.NextEndpoint(fooService, nil) if err == nil || len(endpoint) != 0 { t.Errorf("Didn't fail with non-existent service") } // but bar is still there, and we continue RR from where we left off. - shuffledBarEndpoints = loadBalancer.services[servicePort{barService, ""}].endpoints - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[1], client2) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[1], client2) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) - expectEndpoint(t, loadBalancer, barService, "", shuffledBarEndpoints[0], client1) + shuffledBarEndpoints = loadBalancer.services[barService].endpoints + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[1], client2) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[1], client2) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) + expectEndpoint(t, loadBalancer, barService, shuffledBarEndpoints[0], client1) } diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index 3455c3dce9e..1e51b3d5f6c 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -576,7 +576,6 @@ func TestEtcdUpdateService(t *testing.T) { Selector: map[string]string{ "baz": "bar", }, - Protocol: "TCP", SessionAffinity: "None", }, } diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 0949146af71..8fd79d0003a 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -281,10 +281,12 @@ func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service if rs.cloud == nil { return fmt.Errorf("requested an external service, but no cloud provider supplied.") } - if service.Spec.Protocol != api.ProtocolTCP { - // TODO: Support UDP here too. - return fmt.Errorf("external load balancers for non TCP services are not currently supported.") + + ports, err := getTCPPorts(service) + if err != nil { + return err } + balancer, ok := rs.cloud.TCPLoadBalancer() if !ok { return fmt.Errorf("the cloud provider does not support external TCP load balancers.") @@ -302,18 +304,17 @@ func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service return err } name := rs.getLoadbalancerName(ctx, service) - // TODO: We should be able to rely on valid input, and not do defaulting here. var affinityType api.AffinityType = service.Spec.SessionAffinity if len(service.Spec.PublicIPs) > 0 { for _, publicIP := range service.Spec.PublicIPs { - _, err = balancer.CreateTCPLoadBalancer(name, zone.Region, net.ParseIP(publicIP), service.Spec.Port, hostsFromMinionList(hosts), affinityType) + _, err = balancer.CreateTCPLoadBalancer(name, zone.Region, net.ParseIP(publicIP), ports, hostsFromMinionList(hosts), affinityType) if err != nil { // TODO: have to roll-back any successful calls. return err } } } else { - endpoint, err := balancer.CreateTCPLoadBalancer(name, zone.Region, nil, service.Spec.Port, hostsFromMinionList(hosts), affinityType) + endpoint, err := balancer.CreateTCPLoadBalancer(name, zone.Region, nil, ports, hostsFromMinionList(hosts), affinityType) if err != nil { return err } @@ -322,6 +323,39 @@ func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service return nil } +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 +} + +func portsEqual(x, y *api.Service) bool { + xPorts, err := getTCPPorts(x) + if err != nil { + return false + } + yPorts, err := getTCPPorts(y) + if err != nil { + return false + } + if len(xPorts) != len(yPorts) { + return false + } + for i := range xPorts { + if xPorts[i] != yPorts[i] { + return false + } + } + return true +} + func (rs *REST) deleteExternalLoadBalancer(ctx api.Context, service *api.Service) error { if rs.cloud == nil { return fmt.Errorf("requested an external service, but no cloud provider supplied.") @@ -348,21 +382,21 @@ func (rs *REST) deleteExternalLoadBalancer(ctx api.Context, service *api.Service return nil } -func externalLoadBalancerNeedsUpdate(old, new *api.Service) bool { - if !old.Spec.CreateExternalLoadBalancer && !new.Spec.CreateExternalLoadBalancer { +func externalLoadBalancerNeedsUpdate(oldService, newService *api.Service) bool { + if !oldService.Spec.CreateExternalLoadBalancer && !newService.Spec.CreateExternalLoadBalancer { return false } - if old.Spec.CreateExternalLoadBalancer != new.Spec.CreateExternalLoadBalancer || - old.Spec.Port != new.Spec.Port || - old.Spec.SessionAffinity != new.Spec.SessionAffinity || - old.Spec.Protocol != new.Spec.Protocol { + if oldService.Spec.CreateExternalLoadBalancer != newService.Spec.CreateExternalLoadBalancer { return true } - if len(old.Spec.PublicIPs) != len(new.Spec.PublicIPs) { + if !portsEqual(oldService, newService) || oldService.Spec.SessionAffinity != newService.Spec.SessionAffinity { return true } - for i := range old.Spec.PublicIPs { - if old.Spec.PublicIPs[i] != new.Spec.PublicIPs[i] { + if len(oldService.Spec.PublicIPs) != len(newService.Spec.PublicIPs) { + return true + } + for i := range oldService.Spec.PublicIPs { + if oldService.Spec.PublicIPs[i] != newService.Spec.PublicIPs[i] { return true } } diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 0362247d3a4..6857af95f67 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -17,6 +17,8 @@ limitations under the License. package service import ( + "bytes" + "encoding/gob" "fmt" "net" "strings" @@ -52,6 +54,16 @@ func makeIPNet(t *testing.T) *net.IPNet { return net } +func deepCloneService(svc *api.Service) *api.Service { + buff := new(bytes.Buffer) + enc := gob.NewEncoder(buff) + dec := gob.NewDecoder(buff) + enc.Encode(svc) + result := new(api.Service) + dec.Decode(result) + return result +} + func TestServiceRegistryCreate(t *testing.T) { storage, registry, fakeCloud := NewTestREST(t, nil) storage.portalMgr.randomAttempts = 0 @@ -59,14 +71,19 @@ func TestServiceRegistryCreate(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() - created_svc, _ := storage.Create(ctx, svc) + created_svc, err := storage.Create(ctx, svc) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } created_service := created_svc.(*api.Service) if !api.HasObjectMetaSystemFieldValues(&created_service.ObjectMeta) { t.Errorf("storage did not populate object meta field values") @@ -98,18 +115,22 @@ func TestServiceStorageValidatesCreate(t *testing.T) { "empty ID": { ObjectMeta: api.ObjectMeta{Name: ""}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }, "empty port": { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Protocol: api.ProtocolTCP, + }}, }, }, } @@ -122,7 +143,6 @@ func TestServiceStorageValidatesCreate(t *testing.T) { if !errors.IsInvalid(err) { t.Errorf("Expected to get an invalid resource error, got %v", err) } - } } @@ -132,8 +152,11 @@ func TestServiceRegistryUpdate(t *testing.T) { svc, err := registry.CreateService(ctx, &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz1"}, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }) @@ -145,10 +168,12 @@ func TestServiceRegistryUpdate(t *testing.T) { Name: "foo", ResourceVersion: svc.ResourceVersion}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz2"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }) if err != nil { @@ -175,27 +200,34 @@ func TestServiceStorageValidatesUpdate(t *testing.T) { registry.CreateService(ctx, &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }) failureCases := map[string]api.Service{ "empty ID": { ObjectMeta: api.ObjectMeta{Name: ""}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }, "invalid selector": { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"ThisSelectorFailsValidation": "ok"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }, } @@ -216,14 +248,18 @@ func TestServiceRegistryExternalService(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, CreateExternalLoadBalancer: true, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } - storage.Create(ctx, svc) + if _, err := storage.Create(ctx, svc); err != nil { + t.Fatalf("Unexpected error: %v", err) + } if len(fakeCloud.Calls) != 2 || fakeCloud.Calls[0] != "get-zone" || fakeCloud.Calls[1] != "create" { t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) } @@ -234,7 +270,7 @@ func TestServiceRegistryExternalService(t *testing.T) { if srv == nil { t.Errorf("Failed to find service: %s", svc.Name) } - if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Port != 6502 { + if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Ports[0] != 6502 { t.Errorf("Unexpected balancer created: %v", fakeCloud.Balancers) } } @@ -245,15 +281,19 @@ func TestServiceRegistryExternalServiceError(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, CreateExternalLoadBalancer: true, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() - storage.Create(ctx, svc) + if _, err := storage.Create(ctx, svc); err == nil { + t.Fatalf("Unexpected success") + } if len(fakeCloud.Calls) != 1 || fakeCloud.Calls[0] != "get-zone" { t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) } @@ -269,8 +309,11 @@ func TestServiceRegistryDelete(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } registry.CreateService(ctx, svc) @@ -291,8 +334,11 @@ func TestServiceRegistryDeleteExternal(t *testing.T) { Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, CreateExternalLoadBalancer: true, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } registry.CreateService(ctx, svc) @@ -313,32 +359,80 @@ func TestServiceRegistryUpdateExternalService(t *testing.T) { svc1 := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ - Port: 6502, Selector: map[string]string{"bar": "baz"}, CreateExternalLoadBalancer: false, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } - storage.Create(ctx, svc1) + if _, err := storage.Create(ctx, svc1); err != nil { + t.Fatalf("Unexpected error: %v", err) + } if len(fakeCloud.Calls) != 0 { t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) } // Modify load balancer to be external. - svc2 := new(api.Service) - *svc2 = *svc1 + svc2 := deepCloneService(svc1) svc2.Spec.CreateExternalLoadBalancer = true - storage.Update(ctx, svc2) + if _, _, err := storage.Update(ctx, svc2); err != nil { + t.Fatalf("Unexpected error: %v", err) + } if len(fakeCloud.Calls) != 2 || fakeCloud.Calls[0] != "get-zone" || fakeCloud.Calls[1] != "create" { t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) } // Change port. - svc3 := new(api.Service) - *svc3 = *svc2 - svc3.Spec.Port = 6504 - storage.Update(ctx, svc3) + svc3 := deepCloneService(svc2) + svc3.Spec.Ports[0].Port = 6504 + if _, _, err := storage.Update(ctx, svc3); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(fakeCloud.Calls) != 6 || fakeCloud.Calls[0] != "get-zone" || fakeCloud.Calls[1] != "create" || + fakeCloud.Calls[2] != "get-zone" || fakeCloud.Calls[3] != "delete" || + fakeCloud.Calls[4] != "get-zone" || fakeCloud.Calls[5] != "create" { + t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) + } +} + +func TestServiceRegistryUpdateMultiPortExternalService(t *testing.T) { + ctx := api.NewDefaultContext() + storage, _, fakeCloud := NewTestREST(t, nil) + + // Create external load balancer. + svc1 := &api.Service{ + ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + CreateExternalLoadBalancer: true, + SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Name: "p", + Port: 6502, + Protocol: api.ProtocolTCP, + }, { + Name: "q", + Port: 8086, + Protocol: api.ProtocolTCP, + }}, + }, + } + if _, err := storage.Create(ctx, svc1); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(fakeCloud.Calls) != 2 || fakeCloud.Calls[0] != "get-zone" || fakeCloud.Calls[1] != "create" { + t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) + } + + // Modify ports + svc2 := deepCloneService(svc1) + svc2.Spec.Ports[1].Port = 8088 + if _, _, err := storage.Update(ctx, svc2); err != nil { + t.Fatalf("Unexpected error: %v", err) + } if len(fakeCloud.Calls) != 6 || fakeCloud.Calls[0] != "get-zone" || fakeCloud.Calls[1] != "create" || fakeCloud.Calls[2] != "get-zone" || fakeCloud.Calls[3] != "delete" || fakeCloud.Calls[4] != "get-zone" || fakeCloud.Calls[5] != "create" { @@ -468,9 +562,11 @@ func TestServiceRegistryIPAllocation(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() @@ -487,9 +583,11 @@ func TestServiceRegistryIPAllocation(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "bar"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }} ctx = api.NewDefaultContext() created_svc2, _ := rest.Create(ctx, svc2) @@ -506,9 +604,11 @@ func TestServiceRegistryIPAllocation(t *testing.T) { Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, PortalIP: "1.2.3.93", - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx = api.NewDefaultContext() @@ -527,9 +627,11 @@ func TestServiceRegistryIPReallocation(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() @@ -548,9 +650,11 @@ func TestServiceRegistryIPReallocation(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "bar"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx = api.NewDefaultContext() @@ -572,33 +676,34 @@ func TestServiceRegistryIPUpdate(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() created_svc, _ := rest.Create(ctx, svc) created_service := created_svc.(*api.Service) - if created_service.Spec.Port != 6502 { - t.Errorf("Expected port 6502, but got %v", created_service.Spec.Port) + if created_service.Spec.Ports[0].Port != 6502 { + t.Errorf("Expected port 6502, but got %v", created_service.Spec.Ports[0].Port) } if created_service.Spec.PortalIP != "1.2.3.1" { t.Errorf("Unexpected PortalIP: %s", created_service.Spec.PortalIP) } - update := new(api.Service) - *update = *created_service - update.Spec.Port = 6503 + update := deepCloneService(created_service) + update.Spec.Ports[0].Port = 6503 updated_svc, _, _ := rest.Update(ctx, update) updated_service := updated_svc.(*api.Service) - if updated_service.Spec.Port != 6503 { - t.Errorf("Expected port 6503, but got %v", updated_service.Spec.Port) + if updated_service.Spec.Ports[0].Port != 6503 { + t.Errorf("Expected port 6503, but got %v", updated_service.Spec.Ports[0].Port) } - *update = *created_service - update.Spec.Port = 6503 + update = deepCloneService(created_service) + update.Spec.Ports[0].Port = 6503 update.Spec.PortalIP = "1.2.3.76" // error _, _, err := rest.Update(ctx, update) @@ -614,31 +719,32 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) { svc := &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ - Selector: map[string]string{"bar": "baz"}, - Port: 6502, + Selector: map[string]string{"bar": "baz"}, CreateExternalLoadBalancer: true, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() created_svc, _ := rest.Create(ctx, svc) created_service := created_svc.(*api.Service) - if created_service.Spec.Port != 6502 { - t.Errorf("Expected port 6502, but got %v", created_service.Spec.Port) + if created_service.Spec.Ports[0].Port != 6502 { + t.Errorf("Expected port 6502, but got %v", created_service.Spec.Ports[0].Port) } if created_service.Spec.PortalIP != "1.2.3.1" { t.Errorf("Unexpected PortalIP: %s", created_service.Spec.PortalIP) } - update := new(api.Service) - *update = *created_service + update := deepCloneService(created_service) _, _, err := rest.Update(ctx, update) if err != nil { t.Errorf("Unexpected error %v", err) } - if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Port != 6502 { + if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Ports[0] != 6502 { t.Errorf("Unexpected balancer created: %v", fakeCloud.Balancers) } } @@ -656,9 +762,11 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } ctx := api.NewDefaultContext() @@ -667,9 +775,11 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } rest1.Create(ctx, svc) @@ -683,9 +793,11 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: api.ProtocolTCP, SessionAffinity: api.AffinityTypeNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, } created_svc, _ := rest2.Create(ctx, svc) @@ -743,9 +855,11 @@ func TestCreate(t *testing.T) { Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, PortalIP: "None", - Port: 6502, - Protocol: "TCP", SessionAffinity: "None", + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }, // invalid @@ -756,10 +870,12 @@ func TestCreate(t *testing.T) { &api.Service{ Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, - Port: 6502, - Protocol: "TCP", PortalIP: "invalid", SessionAffinity: "None", + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + }}, }, }, ) diff --git a/pkg/service/endpoints_controller.go b/pkg/service/endpoints_controller.go index 2a12148342d..964dec313bb 100644 --- a/pkg/service/endpoints_controller.go +++ b/pkg/service/endpoints_controller.go @@ -73,44 +73,48 @@ func (e *EndpointController) SyncServiceEndpoints() error { for i := range pods.Items { pod := &pods.Items[i] - // TODO: Once v1beta1 and v1beta2 are EOL'ed, this can - // assume that service.Spec.TargetPort is populated. - _ = v1beta1.Dependency - _ = v1beta2.Dependency - // TODO: Add multiple-ports to Service and expose them here. - portName := "" - portProto := service.Spec.Protocol - portNum, err := findPort(pod, service) - if err != nil { - glog.Errorf("Failed to find port for service %s/%s: %v", service.Namespace, service.Name, err) - continue - } - if len(pod.Status.PodIP) == 0 { - glog.Errorf("Failed to find an IP for pod %s/%s", pod.Namespace, pod.Name) - continue - } + for i := range service.Spec.Ports { + servicePort := &service.Spec.Ports[i] - inService := false - for _, c := range pod.Status.Conditions { - if c.Type == api.PodReady && c.Status == api.ConditionTrue { - inService = true - break + // TODO: Once v1beta1 and v1beta2 are EOL'ed, this can + // assume that service.Spec.TargetPort is populated. + _ = v1beta1.Dependency + _ = v1beta2.Dependency + + portName := servicePort.Name + portProto := servicePort.Protocol + portNum, err := findPort(pod, servicePort) + if err != nil { + glog.Errorf("Failed to find port for service %s/%s: %v", service.Namespace, service.Name, err) + continue + } + if len(pod.Status.PodIP) == 0 { + glog.Errorf("Failed to find an IP for pod %s/%s", pod.Namespace, pod.Name) + continue } - } - if !inService { - glog.V(5).Infof("Pod is out of service: %v/%v", pod.Namespace, pod.Name) - continue - } - epp := api.EndpointPort{Name: portName, Port: portNum, Protocol: portProto} - epa := api.EndpointAddress{IP: pod.Status.PodIP, TargetRef: &api.ObjectReference{ - Kind: "Pod", - Namespace: pod.ObjectMeta.Namespace, - Name: pod.ObjectMeta.Name, - UID: pod.ObjectMeta.UID, - ResourceVersion: pod.ObjectMeta.ResourceVersion, - }} - subsets = append(subsets, api.EndpointSubset{Addresses: []api.EndpointAddress{epa}, Ports: []api.EndpointPort{epp}}) + inService := false + for _, c := range pod.Status.Conditions { + if c.Type == api.PodReady && c.Status == api.ConditionTrue { + inService = true + break + } + } + if !inService { + glog.V(5).Infof("Pod is out of service: %v/%v", pod.Namespace, pod.Name) + continue + } + + epp := api.EndpointPort{Name: portName, Port: portNum, Protocol: portProto} + epa := api.EndpointAddress{IP: pod.Status.PodIP, TargetRef: &api.ObjectReference{ + Kind: "Pod", + Namespace: pod.ObjectMeta.Namespace, + Name: pod.ObjectMeta.Name, + UID: pod.ObjectMeta.UID, + ResourceVersion: pod.ObjectMeta.ResourceVersion, + }} + subsets = append(subsets, api.EndpointSubset{Addresses: []api.EndpointAddress{epa}, Ports: []api.EndpointPort{epp}}) + } } subsets = endpoints.RepackSubsets(subsets) @@ -169,24 +173,24 @@ func findDefaultPort(pod *api.Pod, servicePort int, proto api.Protocol) int { // the service's port. If the targetPort is a non-empty string, look that // string up in all named ports in all containers in the target pod. If no // match is found, fail. -func findPort(pod *api.Pod, service *api.Service) (int, error) { - portName := service.Spec.TargetPort +func findPort(pod *api.Pod, svcPort *api.ServicePort) (int, error) { + portName := svcPort.TargetPort switch portName.Kind { case util.IntstrString: if len(portName.StrVal) == 0 { - return findDefaultPort(pod, service.Spec.Port, service.Spec.Protocol), nil + return findDefaultPort(pod, svcPort.Port, svcPort.Protocol), nil } name := portName.StrVal for _, container := range pod.Spec.Containers { for _, port := range container.Ports { - if port.Name == name && port.Protocol == service.Spec.Protocol { + if port.Name == name && port.Protocol == svcPort.Protocol { return port.ContainerPort, nil } } } case util.IntstrInt: if portName.IntVal == 0 { - return findDefaultPort(pod, service.Spec.Port, service.Spec.Protocol), nil + return findDefaultPort(pod, svcPort.Port, svcPort.Protocol), nil } return portName.IntVal, nil } diff --git a/pkg/service/endpoints_controller_test.go b/pkg/service/endpoints_controller_test.go index 70900e7d926..59de0b591be 100644 --- a/pkg/service/endpoints_controller_test.go +++ b/pkg/service/endpoints_controller_test.go @@ -51,7 +51,8 @@ func newPodList(nPods int, nPorts int) *api.PodList { }, } for j := 0; j < nPorts; j++ { - p.Spec.Containers[0].Ports = append(p.Spec.Containers[0].Ports, api.ContainerPort{ContainerPort: 8080 + j}) + p.Spec.Containers[0].Ports = append(p.Spec.Containers[0].Ports, + api.ContainerPort{Name: fmt.Sprintf("port%d", i), ContainerPort: 8080 + j}) } pods = append(pods, p) } @@ -203,7 +204,7 @@ func TestFindPort(t *testing.T) { for _, tc := range testCases { port, err := findPort(&api.Pod{Spec: api.PodSpec{Containers: tc.containers}}, - &api.Service{Spec: api.ServiceSpec{Protocol: "TCP", Port: servicePort, TargetPort: tc.port}}) + &api.ServicePort{Protocol: "TCP", Port: servicePort, TargetPort: tc.port}) if err != nil && tc.pass { t.Errorf("unexpected error for %s: %v", tc.name, err) } @@ -277,7 +278,7 @@ func TestSyncEndpointsItemsPreserveNoSelector(t *testing.T) { Items: []api.Service{ { ObjectMeta: api.ObjectMeta{Name: "foo"}, - Spec: api.ServiceSpec{}, + Spec: api.ServiceSpec{Ports: []api.ServicePort{{Port: 80}}}, }, }, } @@ -310,7 +311,7 @@ func TestSyncEndpointsProtocolTCP(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}, Spec: api.ServiceSpec{ Selector: map[string]string{}, - Protocol: api.ProtocolTCP, + Ports: []api.ServicePort{{Port: 80}}, }, }, }, @@ -344,7 +345,7 @@ func TestSyncEndpointsProtocolUDP(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}, Spec: api.ServiceSpec{ Selector: map[string]string{}, - Protocol: api.ProtocolUDP, + Ports: []api.ServicePort{{Port: 80}}, }, }, }, @@ -378,6 +379,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}, Spec: api.ServiceSpec{ Selector: map[string]string{}, + Ports: []api.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8080)}}, }, }, }, @@ -417,9 +419,8 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "bar"}, Spec: api.ServiceSpec{ - Selector: map[string]string{ - "foo": "bar", - }, + Selector: map[string]string{"foo": "bar"}, + Ports: []api.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8080)}}, }, }, }, @@ -462,9 +463,8 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Selector: map[string]string{ - "foo": "bar", - }, + Selector: map[string]string{"foo": "bar"}, + Ports: []api.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8080)}}, }, }, }, @@ -496,8 +496,10 @@ func TestSyncEndpointsItems(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}, Spec: api.ServiceSpec{ - Selector: map[string]string{ - "foo": "bar", + Selector: map[string]string{"foo": "bar"}, + Ports: []api.ServicePort{ + {Name: "port0", Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8080)}, + {Name: "port1", Port: 88, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8088)}, }, }, }, @@ -520,7 +522,8 @@ func TestSyncEndpointsItems(t *testing.T) { {IP: "1.2.3.6", TargetRef: &api.ObjectReference{Kind: "Pod", Name: "pod2"}}, }, Ports: []api.EndpointPort{ - {Port: 8080, Protocol: "TCP"}, + {Name: "port0", Port: 8080, Protocol: "TCP"}, + {Name: "port1", Port: 8088, Protocol: "TCP"}, }, }} data := runtime.EncodeOrDie(testapi.Codec(), &api.Endpoints{ @@ -540,9 +543,8 @@ func TestSyncEndpointsPodError(t *testing.T) { { ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ - Selector: map[string]string{ - "foo": "bar", - }, + Selector: map[string]string{"foo": "bar"}, + Ports: []api.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(8080)}}, }, }, }, diff --git a/pkg/types/testing.go b/pkg/types/testing.go deleted file mode 100644 index 1a8c87c529c..00000000000 --- a/pkg/types/testing.go +++ /dev/null @@ -1,26 +0,0 @@ -/* -Copyright 2015 Google Inc. All rights reserved. - -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 types - -import "fmt" - -func NewNamespacedNameOrDie(namespace, name string) (ret NamespacedName) { - if len(namespace) == 0 || len(name) == 0 { - panic(fmt.Sprintf("invalid call to NewNamespacedNameOrDie(%q, %q)", namespace, name)) - } - return NamespacedName{namespace, name} -} diff --git a/pkg/util/hash_test.go b/pkg/util/hash_test.go index db766811cec..4397dfd9ec7 100644 --- a/pkg/util/hash_test.go +++ b/pkg/util/hash_test.go @@ -17,10 +17,106 @@ limitations under the License. package util import ( + "fmt" "hash/adler32" "testing" + + "github.com/davecgh/go-spew/spew" ) +type A struct { + x int + y string +} + +type B struct { + x []int + y map[string]bool +} + +type C struct { + x int + y string +} + +func (c C) String() string { + return fmt.Sprintf("%d:%s", c.x, c.y) +} + +func TestDeepHashObject(t *testing.T) { + // These types are known to fail object hashing because of how spew implements map keys. + // http://github.com/davecgh/go-spew/issues/30 + failureCases := []func() interface{}{ + func() interface{} { return map[A]bool{A{8675309, "Jenny"}: true, A{9765683, "!Jenny"}: false} }, + func() interface{} { return map[C]bool{C{8675309, "Jenny"}: true, C{9765683, "!Jenny"}: false} }, + func() interface{} { return map[*A]bool{&A{8675309, "Jenny"}: true, &A{9765683, "!Jenny"}: false} }, + func() interface{} { return map[*C]bool{&C{8675309, "Jenny"}: true, &C{9765683, "!Jenny"}: false} }, + } + for _, tc := range failureCases { + hasher := adler32.New() + DeepHashObject(hasher, tc()) + first := hasher.Sum32() + alwaysSame := false + for i := 0; i < 100; i++ { + DeepHashObject(hasher, tc()) + if hasher.Sum32() != first { + alwaysSame = false + break + } + } + if alwaysSame { + t.Errorf("expected failure for %q", toString(tc())) + } + } + + successCases := []func() interface{}{ + func() interface{} { return 8675309 }, + func() interface{} { return "Jenny, I got your number" }, + func() interface{} { return []string{"eight", "six", "seven"} }, + func() interface{} { return [...]int{5, 3, 0, 9} }, + func() interface{} { return map[int]string{8: "8", 6: "6", 7: "7"} }, + func() interface{} { return map[string]int{"5": 5, "3": 3, "0": 0, "9": 9} }, + func() interface{} { return A{867, "5309"} }, + func() interface{} { return &A{867, "5309"} }, + func() interface{} { + return B{[]int{8, 6, 7}, map[string]bool{"5": true, "3": true, "0": true, "9": true}} + }, + } + + for _, tc := range successCases { + hasher1 := adler32.New() + DeepHashObject(hasher1, tc()) + hash1 := hasher1.Sum32() + DeepHashObject(hasher1, tc()) + hash2 := hasher1.Sum32() + if hash1 != hash2 { + t.Fatalf("hash of the same object (%q) produced different results: %d vs %d", toString(tc()), hash1, hash2) + } + for i := 0; i < 100; i++ { + hasher2 := adler32.New() + + DeepHashObject(hasher1, tc()) + hash1a := hasher1.Sum32() + DeepHashObject(hasher2, tc()) + hash2a := hasher2.Sum32() + + if hash1a != hash1 { + t.Errorf("repeated hash of the same object (%q) produced different results: %d vs %d", toString(tc()), hash1, hash1a) + } + if hash2a != hash2 { + t.Errorf("repeated hash of the same object (%q) produced different results: %d vs %d", toString(tc()), hash2, hash2a) + } + if hash1a != hash2a { + t.Errorf("hash of the same object produced (%q) different results: %d vs %d", toString(tc()), hash1a, hash2a) + } + } + } +} + +func toString(obj interface{}) string { + return spew.Sprintf("%#v", obj) +} + type wheel struct { radius uint32 } diff --git a/test/e2e/networking.go b/test/e2e/networking.go index 5a5af6c781c..7a2afd34616 100644 --- a/test/e2e/networking.go +++ b/test/e2e/networking.go @@ -77,8 +77,11 @@ var _ = Describe("Networking", func() { }, }, Spec: api.ServiceSpec{ - Port: 8080, - TargetPort: util.NewIntOrStringFromInt(8080), + Ports: []api.ServicePort{{ + Protocol: "TCP", + Port: 8080, + TargetPort: util.NewIntOrStringFromInt(8080), + }}, Selector: map[string]string{ "name": name, }, diff --git a/test/e2e/pods.go b/test/e2e/pods.go index 9d16a950bad..7e9812dff4e 100644 --- a/test/e2e/pods.go +++ b/test/e2e/pods.go @@ -307,8 +307,10 @@ var _ = Describe("Pods", func() { }, }, Spec: api.ServiceSpec{ - Port: 8765, - TargetPort: util.NewIntOrStringFromInt(8080), + Ports: []api.ServicePort{{ + Port: 8765, + TargetPort: util.NewIntOrStringFromInt(8080), + }}, Selector: map[string]string{ "name": serverName, }, diff --git a/test/e2e/service.go b/test/e2e/service.go index c93925d1399..661e0dd820b 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -204,9 +204,11 @@ var _ = Describe("Services", func() { Name: serviceName, }, Spec: api.ServiceSpec{ - Port: 80, - Selector: labels, - TargetPort: util.NewIntOrStringFromInt(80), + Selector: labels, + Ports: []api.ServicePort{{ + Port: 80, + TargetPort: util.NewIntOrStringFromInt(80), + }}, }, } _, err := c.Services(ns).Create(service) @@ -264,9 +266,11 @@ var _ = Describe("Services", func() { Name: serviceName, }, Spec: api.ServiceSpec{ - Port: 80, - Selector: labels, - TargetPort: util.NewIntOrStringFromInt(80), + Selector: labels, + Ports: []api.ServicePort{{ + Port: 80, + TargetPort: util.NewIntOrStringFromInt(80), + }}, CreateExternalLoadBalancer: true, }, } @@ -287,7 +291,7 @@ var _ = Describe("Services", func() { Failf("got unexpected number (%d) of public IPs for externally load balanced service: %v", result.Spec.PublicIPs, result) } ip := result.Spec.PublicIPs[0] - port := result.Spec.Port + port := result.Spec.Ports[0].Port pod := &api.Pod{ TypeMeta: api.TypeMeta{ @@ -351,9 +355,11 @@ var _ = Describe("Services", func() { service := &api.Service{ ObjectMeta: api.ObjectMeta{}, Spec: api.ServiceSpec{ - Port: 80, - Selector: labels, - TargetPort: util.NewIntOrStringFromInt(80), + Selector: labels, + Ports: []api.ServicePort{{ + Port: 80, + TargetPort: util.NewIntOrStringFromInt(80), + }}, CreateExternalLoadBalancer: true, }, }