From 3cbb6b0104d850d4cdf84497a018c4aed87b35b7 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Tue, 24 May 2016 14:56:12 +0200 Subject: [PATCH] kubectl: move printObjectSpecificMessage in factory --- pkg/kubectl/cmd/cmd_test.go | 1 + pkg/kubectl/cmd/create.go | 49 +-------------- pkg/kubectl/cmd/create_test.go | 94 ---------------------------- pkg/kubectl/cmd/replace.go | 4 +- pkg/kubectl/cmd/util/factory.go | 44 +++++++++++++ pkg/kubectl/cmd/util/factory_test.go | 91 +++++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 144 deletions(-) diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 48b5e8d9155..729827772a1 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/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() } diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 3db768c4c1d..1fd923aacb1 100644 --- a/pkg/kubectl/cmd/create.go +++ b/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) diff --git a/pkg/kubectl/cmd/create_test.go b/pkg/kubectl/cmd/create_test.go index e365b406e2f..19653ebeeb5 100644 --- a/pkg/kubectl/cmd/create_test.go +++ b/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) - } - } -} diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index e7157ebb3ef..8b98402aaa8 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/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 }) diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index a3afd44ab6e..3f1c73ed41a 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/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 { diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index c25f7a25b36..4af5fcb8883 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/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) + } + } +}