Merge pull request #26161 from kargakis/move-print-method-in-the-factory

Automatic merge from submit-queue

kubectl: move printObjectSpecificMessage in factory

@smarterclayton @kubernetes/kubectl PTAL

pkg/kubectl/cmd/cmd_test.go

@@ -334,6 +334,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) {
 	f.ProtocolsForObject = rf.ProtocolsForObject
 	f.LabelsForObject = rf.LabelsForObject
 	f.CanBeExposed = rf.CanBeExposed
+	f.PrintObjectSpecificMessage = rf.PrintObjectSpecificMessage
 	return f, t, testapi.Default.Codec()
 }
 

pkg/kubectl/cmd/create.go

@@ -19,17 +19,13 @@ package cmd
 import (
 	"fmt"
 	"io"
-	"strings"
 
 	"github.com/spf13/cobra"
 
-	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/service"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/kubectl"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
-	"k8s.io/kubernetes/pkg/runtime"
 )
 
 // CreateOptions is the start of the data required to perform the operation.  As new fields are added, add them here instead of
@@ -140,7 +136,7 @@ func RunCreate(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *C
 		count++
 		shortOutput := cmdutil.GetFlagString(cmd, "output") == "name"
 		if !shortOutput {
-			printObjectSpecificMessage(info.Object, out)
+			f.PrintObjectSpecificMessage(info.Object, out)
 		}
 		cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, "created")
 		return nil
@@ -154,49 +150,6 @@ func RunCreate(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *C
 	return nil
 }
 
-func printObjectSpecificMessage(obj runtime.Object, out io.Writer) {
-	switch obj := obj.(type) {
-	case *api.Service:
-		if obj.Spec.Type == api.ServiceTypeNodePort {
-			msg := fmt.Sprintf(
-				`You have exposed your service on an external port on all nodes in your
-cluster.  If you want to expose this service to the external internet, you may
-need to set up firewall rules for the service port(s) (%s) to serve traffic.
-
-See http://releases.k8s.io/HEAD/docs/user-guide/services-firewalls.md for more details.
-`,
-				makePortsString(obj.Spec.Ports, true))
-			out.Write([]byte(msg))
-		}
-
-		_, ok := obj.Annotations[service.AnnotationLoadBalancerSourceRangesKey]
-		if ok {
-			msg := fmt.Sprintf(
-				`You are using service annotation [service.beta.kubernetes.io/load-balancer-source-ranges].
-It has been promoted to field [loadBalancerSourceRanges] in service spec. This annotation will be deprecated in the future.
-Please use the loadBalancerSourceRanges field instead.
-
-See http://releases.k8s.io/HEAD/docs/user-guide/services-firewalls.md for more details.
-`)
-			out.Write([]byte(msg))
-		}
-	}
-}
-
-func makePortsString(ports []api.ServicePort, useNodePort bool) string {
-	pieces := make([]string, len(ports))
-	for ix := range ports {
-		var port int32
-		if useNodePort {
-			port = ports[ix].NodePort
-		} else {
-			port = ports[ix].Port
-		}
-		pieces[ix] = fmt.Sprintf("%s:%d", strings.ToLower(string(ports[ix].Protocol)), port)
-	}
-	return strings.Join(pieces, ",")
-}
-
 // createAndRefresh creates an object from input info and refreshes info with that object
 func createAndRefresh(info *resource.Info) error {
 	obj, err := resource.NewHelper(info.Client, info.Mapping).Create(info.Namespace, true, info.Object)

pkg/kubectl/cmd/create_test.go

@@ -21,9 +21,7 @@ import (
 	"net/http"
 	"testing"
 
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/client/unversioned/fake"
-	"k8s.io/kubernetes/pkg/runtime"
 )
 
 func TestExtraArgsFail(t *testing.T) {
@@ -136,95 +134,3 @@ func TestCreateDirectory(t *testing.T) {
 		t.Errorf("unexpected output: %s", buf.String())
 	}
 }
-
-func TestPrintObjectSpecificMessage(t *testing.T) {
-	initTestErrorHandler(t)
-	tests := []struct {
-		obj          runtime.Object
-		expectOutput bool
-	}{
-		{
-			obj:          &api.Service{},
-			expectOutput: false,
-		},
-		{
-			obj:          &api.Pod{},
-			expectOutput: false,
-		},
-		{
-			obj:          &api.Service{Spec: api.ServiceSpec{Type: api.ServiceTypeLoadBalancer}},
-			expectOutput: false,
-		},
-		{
-			obj:          &api.Service{Spec: api.ServiceSpec{Type: api.ServiceTypeNodePort}},
-			expectOutput: true,
-		},
-	}
-	for _, test := range tests {
-		buff := &bytes.Buffer{}
-		printObjectSpecificMessage(test.obj, buff)
-		if test.expectOutput && buff.Len() == 0 {
-			t.Errorf("Expected output, saw none for %v", test.obj)
-		}
-		if !test.expectOutput && buff.Len() > 0 {
-			t.Errorf("Expected no output, saw %s for %v", buff.String(), test.obj)
-		}
-	}
-}
-
-func TestMakePortsString(t *testing.T) {
-	initTestErrorHandler(t)
-	tests := []struct {
-		ports          []api.ServicePort
-		useNodePort    bool
-		expectedOutput string
-	}{
-		{ports: nil, expectedOutput: ""},
-		{ports: []api.ServicePort{}, expectedOutput: ""},
-		{ports: []api.ServicePort{
-			{
-				Port:     80,
-				Protocol: "TCP",
-			},
-		},
-			expectedOutput: "tcp:80",
-		},
-		{ports: []api.ServicePort{
-			{
-				Port:     80,
-				Protocol: "TCP",
-			},
-			{
-				Port:     8080,
-				Protocol: "UDP",
-			},
-			{
-				Port:     9000,
-				Protocol: "TCP",
-			},
-		},
-			expectedOutput: "tcp:80,udp:8080,tcp:9000",
-		},
-		{ports: []api.ServicePort{
-			{
-				Port:     80,
-				NodePort: 9090,
-				Protocol: "TCP",
-			},
-			{
-				Port:     8080,
-				NodePort: 80,
-				Protocol: "UDP",
-			},
-		},
-			useNodePort:    true,
-			expectedOutput: "tcp:9090,udp:80",
-		},
-	}
-	for _, test := range tests {
-		output := makePortsString(test.ports, test.useNodePort)
-		if output != test.expectedOutput {
-			t.Errorf("expected: %s, saw: %s.", test.expectedOutput, output)
-		}
-	}
-}

pkg/kubectl/cmd/replace.go

@@ -151,7 +151,7 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st
 		}
 
 		info.Refresh(obj, true)
-		printObjectSpecificMessage(obj, out)
+		f.PrintObjectSpecificMessage(obj, out)
 		cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, "replaced")
 		return nil
 	})
@@ -244,7 +244,7 @@ func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []
 
 		count++
 		info.Refresh(obj, true)
-		printObjectSpecificMessage(obj, out)
+		f.PrintObjectSpecificMessage(obj, out)
 		cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, "replaced")
 		return nil
 	})

pkg/kubectl/cmd/util/factory.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/kubernetes/federation/apis/federation"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/api/service"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/api/validation"
 	"k8s.io/kubernetes/pkg/apimachinery"
@@ -147,6 +148,8 @@ type Factory struct {
 	// can range over in order to determine if the user has specified an editor
 	// of their choice.
 	EditorEnvs func() []string
+	// PrintObjectSpecificMessage prints object-specific messages on the provided writer
+	PrintObjectSpecificMessage func(obj runtime.Object, out io.Writer)
 }
 
 const (
@@ -747,6 +750,33 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 		EditorEnvs: func() []string {
 			return []string{"KUBE_EDITOR", "EDITOR"}
 		},
+		PrintObjectSpecificMessage: func(obj runtime.Object, out io.Writer) {
+			switch obj := obj.(type) {
+			case *api.Service:
+				if obj.Spec.Type == api.ServiceTypeNodePort {
+					msg := fmt.Sprintf(
+						`You have exposed your service on an external port on all nodes in your
+cluster.  If you want to expose this service to the external internet, you may
+need to set up firewall rules for the service port(s) (%s) to serve traffic.
+
+See http://releases.k8s.io/HEAD/docs/user-guide/services-firewalls.md for more details.
+`,
+						makePortsString(obj.Spec.Ports, true))
+					out.Write([]byte(msg))
+				}
+
+				if _, ok := obj.Annotations[service.AnnotationLoadBalancerSourceRangesKey]; ok {
+					msg := fmt.Sprintf(
+						`You are using service annotation [service.beta.kubernetes.io/load-balancer-source-ranges].
+It has been promoted to field [loadBalancerSourceRanges] in service spec. This annotation will be deprecated in the future.
+Please use the loadBalancerSourceRanges field instead.
+
+See http://releases.k8s.io/HEAD/docs/user-guide/services-firewalls.md for more details.
+`)
+					out.Write([]byte(msg))
+				}
+			}
+		},
 	}
 }
 
@@ -825,6 +855,20 @@ func (f *Factory) BindExternalFlags(flags *pflag.FlagSet) {
 	flags.AddGoFlagSet(flag.CommandLine)
 }
 
+func makePortsString(ports []api.ServicePort, useNodePort bool) string {
+	pieces := make([]string, len(ports))
+	for ix := range ports {
+		var port int32
+		if useNodePort {
+			port = ports[ix].NodePort
+		} else {
+			port = ports[ix].Port
+		}
+		pieces[ix] = fmt.Sprintf("%s:%d", strings.ToLower(string(ports[ix].Protocol)), port)
+	}
+	return strings.Join(pieces, ",")
+}
+
 func getPorts(spec api.PodSpec) []string {
 	result := []string{}
 	for _, container := range spec.Containers {

pkg/kubectl/cmd/util/factory_test.go

@@ -623,3 +623,94 @@ func TestGetFirstPod(t *testing.T) {
 		}
 	}
 }
+
+func TestPrintObjectSpecificMessage(t *testing.T) {
+	f := NewFactory(nil)
+	tests := []struct {
+		obj          runtime.Object
+		expectOutput bool
+	}{
+		{
+			obj:          &api.Service{},
+			expectOutput: false,
+		},
+		{
+			obj:          &api.Pod{},
+			expectOutput: false,
+		},
+		{
+			obj:          &api.Service{Spec: api.ServiceSpec{Type: api.ServiceTypeLoadBalancer}},
+			expectOutput: false,
+		},
+		{
+			obj:          &api.Service{Spec: api.ServiceSpec{Type: api.ServiceTypeNodePort}},
+			expectOutput: true,
+		},
+	}
+	for _, test := range tests {
+		buff := &bytes.Buffer{}
+		f.PrintObjectSpecificMessage(test.obj, buff)
+		if test.expectOutput && buff.Len() == 0 {
+			t.Errorf("Expected output, saw none for %v", test.obj)
+		}
+		if !test.expectOutput && buff.Len() > 0 {
+			t.Errorf("Expected no output, saw %s for %v", buff.String(), test.obj)
+		}
+	}
+}
+
+func TestMakePortsString(t *testing.T) {
+	tests := []struct {
+		ports          []api.ServicePort
+		useNodePort    bool
+		expectedOutput string
+	}{
+		{ports: nil, expectedOutput: ""},
+		{ports: []api.ServicePort{}, expectedOutput: ""},
+		{ports: []api.ServicePort{
+			{
+				Port:     80,
+				Protocol: "TCP",
+			},
+		},
+			expectedOutput: "tcp:80",
+		},
+		{ports: []api.ServicePort{
+			{
+				Port:     80,
+				Protocol: "TCP",
+			},
+			{
+				Port:     8080,
+				Protocol: "UDP",
+			},
+			{
+				Port:     9000,
+				Protocol: "TCP",
+			},
+		},
+			expectedOutput: "tcp:80,udp:8080,tcp:9000",
+		},
+		{ports: []api.ServicePort{
+			{
+				Port:     80,
+				NodePort: 9090,
+				Protocol: "TCP",
+			},
+			{
+				Port:     8080,
+				NodePort: 80,
+				Protocol: "UDP",
+			},
+		},
+			useNodePort:    true,
+			expectedOutput: "tcp:9090,udp:80",
+		},
+	}
+	for _, test := range tests {
+		output := makePortsString(test.ports, test.useNodePort)
+		if output != test.expectedOutput {
+			t.Errorf("expected: %s, saw: %s.", test.expectedOutput, output)
+		}
+	}
+}