Merge pull request #15061 from kargakis/fix-dry-run-in-expose

Fix dry run in expose
2025-08-05 10:19:50 +00:00 · 2015-10-12 12:53:04 -07:00 · 2015-10-12 12:53:04 -07:00 · 075c3a2c71
commit 075c3a2c71
parent 80bf32ce56 4b23bf602d
5 changed files with 87 additions and 146 deletions
--- 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
--- 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

 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_expose.md?pixel)]()
--- 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()
 }

--- 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
@ -212,26 +197,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")
--- 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)
 		}
 	}
 }