From 083c5bebbda587930cf79e65e8413f6560dae4a8 Mon Sep 17 00:00:00 2001 From: kargakis Date: Sat, 3 Oct 2015 01:52:42 +0200 Subject: [PATCH 1/2] expose: --dry-run should dump the object Also make some tests use --dry-run and compare generated objects against expected --- pkg/kubectl/cmd/cmd_test.go | 7 +- pkg/kubectl/cmd/expose.go | 36 +++++---- pkg/kubectl/cmd/expose_test.go | 135 ++++++++++++--------------------- 3 files changed, 67 insertions(+), 111 deletions(-) diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 8049734a74e..b7377dcd0db 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -228,12 +228,6 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { ClientConfig: func() (*client.Config, error) { return t.ClientConfig, t.Err }, - CanBeExposed: func(kind string) error { - if kind != "ReplicationController" && kind != "Service" && kind != "Pod" { - return fmt.Errorf("invalid resource provided: %v, only a replication controller, service or pod is accepted", kind) - } - return nil - }, Generator: func(name string) (kubectl.Generator, bool) { generator, ok := generators[name] return generator, ok @@ -241,6 +235,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { } rf := cmdutil.NewFactory(nil) f.PodSelectorForObject = rf.PodSelectorForObject + f.CanBeExposed = rf.CanBeExposed return f, t, testapi.Default.Codec() } diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index df987971183..d7aa96c579f 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -212,26 +212,24 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str } // TODO: extract this flag to a central location, when such a location exists. if cmdutil.GetFlagBool(cmd, "dry-run") { - fmt.Fprintln(out, "running in dry-run mode...") - } else { - // Serialize the configuration into an annotation. - if err := kubectl.UpdateApplyAnnotation(info); err != nil { - return err - } - - // Serialize the object with the annotation applied. - data, err := info.Mapping.Codec.Encode(info.Object) - if err != nil { - return err - } - - object, err = resource.NewHelper(info.Client, info.Mapping).Create(namespace, false, data) - if err != nil { - return err - } + return f.PrintObject(cmd, object, out) } - outputFormat := cmdutil.GetFlagString(cmd, "output") - if outputFormat != "" { + // Serialize the configuration into an annotation. + if err := kubectl.UpdateApplyAnnotation(info); err != nil { + return err + } + + // Serialize the object with the annotation applied. + data, err := info.Mapping.Codec.Encode(object) + if err != nil { + return err + } + object, err = resource.NewHelper(info.Client, info.Mapping).Create(namespace, false, data) + if err != nil { + return err + } + + if len(cmdutil.GetFlagString(cmd, "output")) > 0 { return f.PrintObject(cmd, object, out) } cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "exposed") diff --git a/pkg/kubectl/cmd/expose_test.go b/pkg/kubectl/cmd/expose_test.go index c944b2bdd4c..6a03cac4cac 100644 --- a/pkg/kubectl/cmd/expose_test.go +++ b/pkg/kubectl/cmd/expose_test.go @@ -23,8 +23,8 @@ import ( "testing" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/client/unversioned/fake" + "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" ) @@ -149,7 +149,7 @@ func TestRunExposeService(t *testing.T) { Selector: map[string]string{"app": "go"}, }, }, - flags: map[string]string{"selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test", "create-external-load-balancer": "true"}, + flags: map[string]string{"selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test", "type": "LoadBalancer", "dry-run": "true"}, output: &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "", Labels: map[string]string{"svc": "test"}}, Spec: api.ServiceSpec{ @@ -164,8 +164,7 @@ func TestRunExposeService(t *testing.T) { Type: api.ServiceTypeLoadBalancer, }, }, - expected: "service \"foo\" exposed", - status: 200, + status: 200, }, { name: "expose-affinity-service", @@ -181,7 +180,7 @@ func TestRunExposeService(t *testing.T) { Selector: map[string]string{"app": "go"}, }, }, - flags: map[string]string{"selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test", "create-external-load-balancer": "true", "session-affinity": "ClientIP"}, + flags: map[string]string{"selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test", "type": "LoadBalancer", "session-affinity": "ClientIP", "dry-run": "true"}, output: &api.Service{ ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "", Labels: map[string]string{"svc": "test"}}, Spec: api.ServiceSpec{ @@ -197,8 +196,7 @@ func TestRunExposeService(t *testing.T) { SessionAffinity: api.ServiceAffinityClientIP, }, }, - expected: "service \"foo\" exposed", - status: 200, + status: 200, }, { name: "expose-external-service", @@ -215,7 +213,7 @@ func TestRunExposeService(t *testing.T) { }, }, // Even if we specify --selector, since service/test doesn't need one it will ignore it - flags: map[string]string{"selector": "svc=fromexternal", "port": "90", "labels": "svc=fromexternal", "name": "frombaz", "generator": "service/test"}, + flags: map[string]string{"selector": "svc=fromexternal", "port": "90", "labels": "svc=fromexternal", "name": "frombaz", "generator": "service/test", "dry-run": "true"}, output: &api.Service{ ObjectMeta: api.ObjectMeta{Name: "frombaz", Namespace: "", Labels: map[string]string{"svc": "fromexternal"}}, Spec: api.ServiceSpec{ @@ -226,11 +224,39 @@ func TestRunExposeService(t *testing.T) { TargetPort: util.NewIntOrStringFromInt(90), }, }, - Selector: map[string]string{"svc": "fromexternal"}, }, }, - expected: "service \"frombaz\" exposed", - status: 200, + status: 200, + }, + { + name: "expose-from-file", + args: []string{}, + ns: "test", + calls: map[string]string{ + "GET": "/namespaces/test/services/redis-master", + "POST": "/namespaces/test/services", + }, + input: &api.Service{ + ObjectMeta: api.ObjectMeta{Name: "redis-master", Namespace: "test", ResourceVersion: "12"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"app": "go"}, + }, + }, + flags: map[string]string{"filename": "../../../examples/guestbook/redis-master-service.yaml", "selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test", "dry-run": "true"}, + output: &api.Service{ + ObjectMeta: api.ObjectMeta{Name: "foo", Labels: map[string]string{"svc": "test"}}, + Spec: api.ServiceSpec{ + Ports: []api.ServicePort{ + { + Protocol: api.ProtocolUDP, + Port: 14, + TargetPort: util.NewIntOrStringFromInt(14), + }, + }, + Selector: map[string]string{"func": "stream"}, + }, + }, + status: 200, }, { name: "truncate-name", @@ -264,7 +290,7 @@ func TestRunExposeService(t *testing.T) { for _, test := range tests { f, tf, codec := NewAPIFactory() - tf.Printer = &testPrinter{} + tf.Printer = &kubectl.JSONPrinter{} tf.Client = &fake.RESTClient{ Codec: codec, Client: fake.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { @@ -289,82 +315,19 @@ func TestRunExposeService(t *testing.T) { } cmd.Run(cmd, test.args) - out, expectedOut := buf.String(), test.expected - if !strings.Contains(out, expectedOut) { - t.Errorf("%s: Unexpected output! Expected\n%s\ngot\n%s", test.name, expectedOut, out) - } - } -} - -func TestRunExposeServiceFromFile(t *testing.T) { - test := struct { - calls map[string]string - input runtime.Object - flags map[string]string - output runtime.Object - expected string - status int - }{ - calls: map[string]string{ - "GET": "/namespaces/test/services/redis-master", - "POST": "/namespaces/test/services", - }, - input: &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"}, - TypeMeta: unversioned.TypeMeta{Kind: "Service", APIVersion: "v1"}, - Spec: api.ServiceSpec{ - Selector: map[string]string{"app": "go"}, - }, - }, - flags: map[string]string{"selector": "func=stream", "protocol": "UDP", "port": "14", "name": "foo", "labels": "svc=test"}, - output: &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "test", ResourceVersion: "12", Labels: map[string]string{"svc": "test"}}, - TypeMeta: unversioned.TypeMeta{Kind: "Service", APIVersion: "v1"}, - Spec: api.ServiceSpec{ - Ports: []api.ServicePort{ - { - Name: "default", - Protocol: api.Protocol("UDP"), - Port: 14, - }, - }, - Selector: map[string]string{"func": "stream"}, - }, - }, - status: 200, - } - - f, tf, codec := NewAPIFactory() - tf.Printer = &testPrinter{} - tf.Client = &fake.RESTClient{ - Codec: codec, - Client: fake.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { - switch p, m := req.URL.Path, req.Method; { - case p == test.calls[m] && m == "GET": - return &http.Response{StatusCode: test.status, Body: objBody(codec, test.input)}, nil - case p == test.calls[m] && m == "POST": - return &http.Response{StatusCode: test.status, Body: objBody(codec, test.output)}, nil - default: - t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) - return nil, nil - } - }), - } - tf.Namespace = "test" - buf := bytes.NewBuffer([]byte{}) - - cmd := NewCmdExposeService(f, buf) - cmd.SetOutput(buf) - - for flag, value := range test.flags { - cmd.Flags().Set(flag, value) - } - cmd.Flags().Set("filename", "../../../examples/guestbook/redis-master-service.yaml") - cmd.Run(cmd, []string{}) - if len(test.expected) > 0 { out := buf.String() + if _, ok := test.flags["dry-run"]; ok { + buf.Reset() + if err := tf.Printer.PrintObj(test.output, buf); err != nil { + t.Errorf("%s: Unexpected error: %v", test.name, err) + continue + } + + test.expected = buf.String() + } + if !strings.Contains(out, test.expected) { - t.Errorf("unexpected output: %s", out) + t.Errorf("%s: Unexpected output! Expected\n%s\ngot\n%s", test.name, test.expected, out) } } } From 4b23bf602dcbf39e160f06828afd5f5ca80a6400 Mon Sep 17 00:00:00 2001 From: kargakis Date: Sun, 4 Oct 2015 01:06:32 +0200 Subject: [PATCH 2/2] expose: Minor cleanup --- docs/man/man1/kubectl-expose.1 | 2 +- docs/user-guide/kubectl/kubectl_expose.md | 4 +- pkg/kubectl/cmd/expose.go | 49 ++++++++--------------- 3 files changed, 20 insertions(+), 35 deletions(-) diff --git a/docs/man/man1/kubectl-expose.1 b/docs/man/man1/kubectl-expose.1 index b93b58c4855..840fe4eaaee 100644 --- a/docs/man/man1/kubectl-expose.1 +++ b/docs/man/man1/kubectl-expose.1 @@ -77,7 +77,7 @@ re\-use the labels from the resource it exposes. An inline JSON override for the generated object. If this is non\-empty, it is used to override the generated object. Requires that the object supply a valid apiVersion field. .PP -\fB\-\-port\fP=\-1 +\fB\-\-port\fP="" The port that the service should serve on. Copied from the resource being exposed, if unspecified .PP diff --git a/docs/user-guide/kubectl/kubectl_expose.md b/docs/user-guide/kubectl/kubectl_expose.md index 5351215ac3f..b3a6e5c399d 100644 --- a/docs/user-guide/kubectl/kubectl_expose.md +++ b/docs/user-guide/kubectl/kubectl_expose.md @@ -82,7 +82,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream -o, --output="": Output format. One of: json|yaml|wide|name|go-template=...|go-template-file=...|jsonpath=...|jsonpath-file=... See golang template [http://golang.org/pkg/text/template/#pkg-overview] and jsonpath template [http://releases.k8s.io/HEAD/docs/user-guide/jsonpath.md]. --output-version="": Output the formatted object with the given version (default api-version). --overrides="": An inline JSON override for the generated object. If this is non-empty, it is used to override the generated object. Requires that the object supply a valid apiVersion field. - --port=-1: The port that the service should serve on. Copied from the resource being exposed, if unspecified + --port="": The port that the service should serve on. Copied from the resource being exposed, if unspecified --protocol="TCP": The network protocol for the service to be created. Default is 'tcp'. --selector="": A label selector to use for this service. If empty (the default) infer the selector from the replication controller. --session-affinity="": If non-empty, set the session affinity for the service to this; legal values: 'None', 'ClientIP' @@ -125,7 +125,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-09-22 12:53:42.292344962 +0000 UTC +###### Auto generated by spf13/cobra at 2015-10-10 14:16:24.22183637 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_expose.md?pixel)]() diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index d7aa96c579f..3cc960fdc58 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -73,7 +73,7 @@ func NewCmdExposeService(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmdutil.AddPrinterFlags(cmd) 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. Copied from the resource being exposed, if unspecified") + cmd.Flags().String("port", "", "The port that the service should serve on. Copied from the resource being exposed, if unspecified") cmd.Flags().String("type", "", "Type for this service: ClusterIP, NodePort, or LoadBalancer. Default is 'ClusterIP'.") // TODO: remove create-external-load-balancer in code on or after Aug 25, 2016. cmd.Flags().Bool("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'.") @@ -150,32 +150,22 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str params["selector"] = s } - if cmdutil.GetFlagInt(cmd, "port") < 1 { - noPorts := true - for _, param := range names { - if param.Name == "port" { - noPorts = false - break - } + // For objects that need a port, derive it from the exposed object in case a user + // didn't explicitly specify one via --port + if port, found := params["port"]; found && kubectl.IsZero(port) { + ports, err := f.PortsForObject(inputObject) + if err != nil { + return cmdutil.UsageError(cmd, fmt.Sprintf("couldn't find port via --port flag or introspection: %s", err)) } - if cmdutil.GetFlagInt(cmd, "port") < 0 && !noPorts { - ports, err := f.PortsForObject(inputObject) - if err != nil { - return cmdutil.UsageError(cmd, fmt.Sprintf("couldn't find port via --port flag or introspection: %s", err)) - } - switch len(ports) { - case 0: - return cmdutil.UsageError(cmd, "couldn't find port via --port flag or introspection") - case 1: - params["port"] = ports[0] - default: - return cmdutil.UsageError(cmd, fmt.Sprintf("multiple ports to choose from: %v, please explicitly specify a port using the --port flag.", ports)) - } + switch len(ports) { + case 0: + return cmdutil.UsageError(cmd, "couldn't find port via --port flag or introspection") + case 1: + params["port"] = ports[0] + default: + return cmdutil.UsageError(cmd, fmt.Sprintf("multiple ports to choose from: %v, please explicitly specify a port using the --port flag.", ports)) } } - if cmdutil.GetFlagBool(cmd, "create-external-load-balancer") { - params["create-external-load-balancer"] = "true" - } if kubectl.IsZero(params["labels"]) { labels, err := f.LabelsForObject(inputObject) if err != nil { @@ -183,22 +173,17 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str } params["labels"] = kubectl.MakeLabels(labels) } - if v := cmdutil.GetFlagString(cmd, "type"); v != "" { - params["type"] = v - } - err = kubectl.ValidateParams(names, params) - if err != nil { + if err = kubectl.ValidateParams(names, params); err != nil { return err } - // Expose new object + // Generate new object object, err := generator.Generate(params) if err != nil { return err } - inline := cmdutil.GetFlagString(cmd, "overrides") - if len(inline) > 0 { + if inline := cmdutil.GetFlagString(cmd, "overrides"); len(inline) > 0 { object, err = cmdutil.Merge(object, inline, mapping.Kind) if err != nil { return err