From fa6ddf13e5c70df59c75768631b50f611d02896e Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 4 Feb 2015 12:14:53 -0800 Subject: [PATCH] Fix service generation and add tests I should have included in the first place. --- pkg/kubectl/cmd/expose.go | 5 +- pkg/kubectl/service.go | 15 ++-- pkg/kubectl/service_test.go | 141 ++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 pkg/kubectl/service_test.go diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index 2a7bda235d..956da11f3b 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -51,9 +51,6 @@ $ kubectl expose streamer --port=4100 --protocol=udp --service-name=video-stream client, err := f.Client(cmd) checkErr(err) - rc, err := client.ReplicationControllers(namespace).Get(args[0]) - checkErr(err) - generatorName := GetFlagString(cmd, "generator") generator, found := kubectl.Generators[generatorName] if !found { @@ -70,6 +67,8 @@ $ kubectl expose streamer --port=4100 --protocol=udp --service-name=video-stream params["name"] = GetFlagString(cmd, "service-name") } if _, found := params["selector"]; !found { + rc, err := client.ReplicationControllers(namespace).Get(args[0]) + checkErr(err) params["selector"] = kubectl.MakeLabels(rc.Spec.Selector) } if GetFlagBool(cmd, "create-external-load-balancer") { diff --git a/pkg/kubectl/service.go b/pkg/kubectl/service.go index ce7acf323c..2a8bf62855 100644 --- a/pkg/kubectl/service.go +++ b/pkg/kubectl/service.go @@ -49,7 +49,11 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro if !found { return nil, fmt.Errorf("'name' is a required parameter.") } - port, err := strconv.Atoi(params["port"]) + portString, found := params["port"] + if !found { + return nil, fmt.Errorf("'port' is a required parameter.") + } + port, err := strconv.Atoi(portString) if err != nil { return nil, err } @@ -65,11 +69,10 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro } containerPort, found := params["container-port"] if found && len(containerPort) > 0 { - cPort, err := strconv.Atoi(containerPort) - if err != nil { - service.Spec.ContainerPort = util.NewIntOrStringFromInt(cPort) - } else { + if cPort, err := strconv.Atoi(containerPort); err != nil { service.Spec.ContainerPort = util.NewIntOrStringFromString(containerPort) + } else { + service.Spec.ContainerPort = util.NewIntOrStringFromInt(cPort) } } else { service.Spec.ContainerPort = util.NewIntOrStringFromInt(port) @@ -77,7 +80,7 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro if params["create-external-load-balancer"] == "true" { service.Spec.CreateExternalLoadBalancer = true } - if len(params["public-ip"]) == 0 { + if len(params["public-ip"]) != 0 { service.Spec.PublicIPs = []string{params["public-ip"]} } return &service, nil diff --git a/pkg/kubectl/service_test.go b/pkg/kubectl/service_test.go new file mode 100644 index 0000000000..4b299b0280 --- /dev/null +++ b/pkg/kubectl/service_test.go @@ -0,0 +1,141 @@ +/* +Copyright 2014 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 kubectl + +import ( + "reflect" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" +) + +func TestGenerateService(t *testing.T) { + tests := []struct { + params map[string]string + expected api.Service + }{ + { + params: map[string]string{ + "selector": "foo=bar,baz=blah", + "name": "test", + "port": "80", + "protocol": "TCP", + "container-port": "1234", + }, + expected: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "test", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{ + "foo": "bar", + "baz": "blah", + }, + Port: 80, + Protocol: "TCP", + ContainerPort: util.NewIntOrStringFromInt(1234), + }, + }, + }, + { + params: map[string]string{ + "selector": "foo=bar,baz=blah", + "name": "test", + "port": "80", + "protocol": "UDP", + "container-port": "foobar", + }, + expected: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "test", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{ + "foo": "bar", + "baz": "blah", + }, + Port: 80, + Protocol: "UDP", + ContainerPort: util.NewIntOrStringFromString("foobar"), + }, + }, + }, + { + params: map[string]string{ + "selector": "foo=bar,baz=blah", + "name": "test", + "port": "80", + "protocol": "UDP", + "container-port": "foobar", + "public-ip": "1.2.3.4", + }, + expected: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "test", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{ + "foo": "bar", + "baz": "blah", + }, + Port: 80, + Protocol: "UDP", + PublicIPs: []string{"1.2.3.4"}, + ContainerPort: util.NewIntOrStringFromString("foobar"), + }, + }, + }, + { + params: map[string]string{ + "selector": "foo=bar,baz=blah", + "name": "test", + "port": "80", + "protocol": "UDP", + "container-port": "foobar", + "public-ip": "1.2.3.4", + "create-external-load-balancer": "true", + }, + expected: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "test", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{ + "foo": "bar", + "baz": "blah", + }, + Port: 80, + Protocol: "UDP", + PublicIPs: []string{"1.2.3.4"}, + ContainerPort: util.NewIntOrStringFromString("foobar"), + CreateExternalLoadBalancer: true, + }, + }, + }, + } + generator := ServiceGenerator{} + for _, test := range tests { + obj, err := generator.Generate(test.params) + if !reflect.DeepEqual(obj, &test.expected) { + t.Errorf("expected:\n%#v\ngot\n%#v\n", &test.expected, obj) + } + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } +}