Merge pull request #4124 from brendandburns/e2e2

Fix service generation and add tests I should have included in the first place.

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") {

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 cPort, err := strconv.Atoi(containerPort); err != nil {
-		if err != nil {
-			service.Spec.ContainerPort = util.NewIntOrStringFromInt(cPort)
-		} else {
 			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

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