From 496f30a92f8aa5de849e40533dc86973db3c0a02 Mon Sep 17 00:00:00 2001 From: nikhiljindal Date: Thu, 16 Jul 2015 14:18:17 -0700 Subject: [PATCH] Introduce a new service generator that leaves service port unnamed --- docs/man/man1/kubectl-expose.1 | 4 +- docs/user-guide/kubectl/kubectl_expose.md | 4 +- hack/test-cmd.sh | 19 ++++++--- pkg/kubectl/cmd/cmd_test.go | 3 +- pkg/kubectl/cmd/expose.go | 2 +- pkg/kubectl/cmd/util/factory.go | 3 +- pkg/kubectl/service.go | 34 +++++++++++++-- pkg/kubectl/service_test.go | 52 ++++++++++++++++++----- 8 files changed, 92 insertions(+), 29 deletions(-) diff --git a/docs/man/man1/kubectl-expose.1 b/docs/man/man1/kubectl-expose.1 index dd2a2cc773f..a97013a2758 100644 --- a/docs/man/man1/kubectl-expose.1 +++ b/docs/man/man1/kubectl-expose.1 @@ -35,8 +35,8 @@ re\-use the labels from the resource it exposes. If true, only print the object that would be sent, without creating it. .PP -\fB\-\-generator\fP="service/v1" - The name of the API generator to use. Default is 'service/v1'. +\fB\-\-generator\fP="service/v2" + The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'. .PP \fB\-h\fP, \fB\-\-help\fP=false diff --git a/docs/user-guide/kubectl/kubectl_expose.md b/docs/user-guide/kubectl/kubectl_expose.md index 88e34f94d37..43ccd06cbd5 100644 --- a/docs/user-guide/kubectl/kubectl_expose.md +++ b/docs/user-guide/kubectl/kubectl_expose.md @@ -56,7 +56,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream --container-port="": Synonym for --target-port --create-external-load-balancer=false: If true, create an external load balancer for this service (trumped by --type). Implementation is cloud provider dependent. Default is 'false'. --dry-run=false: If true, only print the object that would be sent, without creating it. - --generator="service/v1": The name of the API generator to use. Default is 'service/v1'. + --generator="service/v2": The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'. -h, --help=false: help for expose -l, --labels="": Labels to apply to the service created by this call. --name="": The name for the newly created object. @@ -105,7 +105,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream ### SEE ALSO * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-07-14 00:11:42.957615868 +0000 UTC +###### Auto generated by spf13/cobra at 2015-07-16 21:22:36.08263026 +0000 UTC diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index c710151a4d3..f5088a7ffb8 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -178,6 +178,7 @@ runTests() { rc_status_replicas_field=".status.replicas" rc_container_image_field=".spec.template.spec.containers" port_field="(index .spec.ports 0).port" + port_name="(index .spec.ports 0).name" image_field="(index .spec.containers 0).image" # Passing no arguments to create is an error @@ -623,20 +624,24 @@ __EOF__ kube::test::get_object_assert 'rc frontend' "{{$rc_replicas_field}}" '3' # Command kubectl expose rc frontend --port=80 "${kube_flags[@]}" - # Post-condition: service exists - kube::test::get_object_assert 'service frontend' "{{$port_field}}" '80' + # Post-condition: service exists and the port is unnamed + kube::test::get_object_assert 'service frontend' "{{$port_name}} {{$port_field}}" ' 80' # Command kubectl expose service frontend --port=443 --name=frontend-2 "${kube_flags[@]}" - # Post-condition: service exists - kube::test::get_object_assert 'service frontend-2' "{{$port_field}}" '443' + # Post-condition: service exists and the port is unnamed + kube::test::get_object_assert 'service frontend-2' "{{$port_name}} {{$port_field}}" ' 443' # Command kubectl create -f docs/user-guide/limitrange/valid-pod.yaml "${kube_flags[@]}" kubectl expose pod valid-pod --port=444 --name=frontend-3 "${kube_flags[@]}" - # Post-condition: service exists - kube::test::get_object_assert 'service frontend-3' "{{$port_field}}" '444' + # Post-condition: service exists and the port is unnamed + kube::test::get_object_assert 'service frontend-3' "{{$port_name}} {{$port_field}}" ' 444' + # Create a service using service/v1 generator + kubectl expose rc frontend --port=80 --name=frontend-4 --generator=service/v1 "${kube_flags[@]}" + # Post-condition: service exists and the port is named default. + kube::test::get_object_assert 'service frontend-4' "{{$port_name}} {{$port_field}}" 'default 80' # Cleanup services kubectl delete pod valid-pod "${kube_flags[@]}" - kubectl delete service frontend{,-2,-3} "${kube_flags[@]}" + kubectl delete service frontend{,-2,-3,-4} "${kube_flags[@]}" ### Perform a rolling update with --image # Command diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 899ce21c3eb..707badf27ca 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -177,7 +177,8 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { } generators := map[string]kubectl.Generator{ "run/v1": kubectl.BasicReplicationController{}, - "service/v1": kubectl.ServiceGenerator{}, + "service/v1": kubectl.ServiceGeneratorV1{}, + "service/v2": kubectl.ServiceGeneratorV2{}, } return &cmdutil.Factory{ Object: func() (meta.RESTMapper, runtime.ObjectTyper) { diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index 53d8f17c792..2ba82daaeb1 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -56,7 +56,7 @@ func NewCmdExposeService(f *cmdutil.Factory, out io.Writer) *cobra.Command { }, } cmdutil.AddPrinterFlags(cmd) - cmd.Flags().String("generator", "service/v1", "The name of the API generator to use. Default is 'service/v1'.") + cmd.Flags().String("generator", "service/v2", "The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'.") cmd.Flags().String("protocol", "TCP", "The network protocol for the service to be created. Default is 'tcp'.") cmd.Flags().Int("port", -1, "The port that the service should serve on. Required.") cmd.MarkFlagRequired("port") diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 4c1a14c1952..ffd2ae1e938 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -97,7 +97,8 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory { generators := map[string]kubectl.Generator{ "run/v1": kubectl.BasicReplicationController{}, - "service/v1": kubectl.ServiceGenerator{}, + "service/v1": kubectl.ServiceGeneratorV1{}, + "service/v2": kubectl.ServiceGeneratorV2{}, } clientConfig := optionalClientConfig diff --git a/pkg/kubectl/service.go b/pkg/kubectl/service.go index 5f41a0da013..ab8e1893aef 100644 --- a/pkg/kubectl/service.go +++ b/pkg/kubectl/service.go @@ -25,9 +25,29 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) -type ServiceGenerator struct{} +// The only difference between ServiceGeneratorV1 and V2 is that the service port is named "default" in V1, while it is left unnamed in V2. +type ServiceGeneratorV1 struct{} -func (ServiceGenerator) ParamNames() []GeneratorParam { +func (ServiceGeneratorV1) ParamNames() []GeneratorParam { + return paramNames() +} + +func (ServiceGeneratorV1) Generate(params map[string]string) (runtime.Object, error) { + params["port-name"] = "default" + return generate(params) +} + +type ServiceGeneratorV2 struct{} + +func (ServiceGeneratorV2) ParamNames() []GeneratorParam { + return paramNames() +} + +func (ServiceGeneratorV2) Generate(params map[string]string) (runtime.Object, error) { + return generate(params) +} + +func paramNames() []GeneratorParam { return []GeneratorParam{ {"default-name", true}, {"name", false}, @@ -40,10 +60,11 @@ func (ServiceGenerator) ParamNames() []GeneratorParam { {"protocol", false}, {"container-port", false}, // alias of target-port {"target-port", false}, + {"port-name", false}, } } -func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, error) { +func generate(params map[string]string) (runtime.Object, error) { selectorString, found := params["selector"] if !found || len(selectorString) == 0 { return nil, fmt.Errorf("'selector' is a required parameter.") @@ -77,6 +98,11 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro if err != nil { return nil, err } + servicePortName, found := params["port-name"] + if !found { + // Leave the port unnamed. + servicePortName = "" + } service := api.Service{ ObjectMeta: api.ObjectMeta{ Name: name, @@ -86,7 +112,7 @@ func (ServiceGenerator) Generate(params map[string]string) (runtime.Object, erro Selector: selector, Ports: []api.ServicePort{ { - Name: "default", + Name: servicePortName, Port: port, Protocol: api.Protocol(params["protocol"]), }, diff --git a/pkg/kubectl/service_test.go b/pkg/kubectl/service_test.go index deaa87777a0..6987843b73f 100644 --- a/pkg/kubectl/service_test.go +++ b/pkg/kubectl/service_test.go @@ -26,10 +26,12 @@ import ( func TestGenerateService(t *testing.T) { tests := []struct { - params map[string]string - expected api.Service + generator Generator + params map[string]string + expected api.Service }{ { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -48,7 +50,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(1234), @@ -58,6 +59,8 @@ func TestGenerateService(t *testing.T) { }, }, { + + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -76,7 +79,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "UDP", TargetPort: util.NewIntOrStringFromString("foobar"), @@ -86,6 +88,7 @@ func TestGenerateService(t *testing.T) { }, }, { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "labels": "key1=value1,key2=value2", @@ -109,7 +112,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(1234), @@ -119,6 +121,7 @@ func TestGenerateService(t *testing.T) { }, }, { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -138,7 +141,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "UDP", TargetPort: util.NewIntOrStringFromString("foobar"), @@ -149,6 +151,7 @@ func TestGenerateService(t *testing.T) { }, }, { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -169,7 +172,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "UDP", TargetPort: util.NewIntOrStringFromString("foobar"), @@ -181,6 +183,7 @@ func TestGenerateService(t *testing.T) { }, }, { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -200,7 +203,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "UDP", TargetPort: util.NewIntOrStringFromString("foobar"), @@ -211,6 +213,7 @@ func TestGenerateService(t *testing.T) { }, }, { + generator: ServiceGeneratorV2{}, params: map[string]string{ "selector": "foo=bar,baz=blah", "name": "test", @@ -231,7 +234,6 @@ func TestGenerateService(t *testing.T) { }, Ports: []api.ServicePort{ { - Name: "default", Port: 80, Protocol: "UDP", TargetPort: util.NewIntOrStringFromString("foobar"), @@ -241,10 +243,38 @@ func TestGenerateService(t *testing.T) { }, }, }, + { + generator: ServiceGeneratorV1{}, + 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", + }, + Ports: []api.ServicePort{ + { + Name: "default", + Port: 80, + Protocol: "TCP", + TargetPort: util.NewIntOrStringFromInt(1234), + }, + }, + }, + }, + }, } - generator := ServiceGenerator{} for _, test := range tests { - obj, err := generator.Generate(test.params) + obj, err := test.generator.Generate(test.params) if !reflect.DeepEqual(obj, &test.expected) { t.Errorf("expected:\n%#v\ngot\n%#v\n", &test.expected, obj) }