Move unrelated methods from the factory to helper

pkg/kubectl/cmd/expose.go

@@ -192,9 +192,10 @@ func (o *ExposeServiceOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) e
 	o.Builder = f.NewBuilder()
 	o.CanBeExposed = polymorphichelpers.CanBeExposedFn
 	o.ClientForMapping = f.ClientForMapping
-	o.MapBasedSelectorForObject = f.MapBasedSelectorForObject
+	o.MapBasedSelectorForObject = polymorphichelpers.MapBasedSelectorForObjectFn
+	o.ProtocolsForObject = polymorphichelpers.ProtocolsForObjectFn
 	o.PortsForObject = polymorphichelpers.PortsForObjectFn
-	o.ProtocolsForObject = f.ProtocolsForObject
+
 	o.Mapper, err = f.ToRESTMapper()
 	if err != nil {
 		return err

pkg/kubectl/cmd/util/BUILD

@@ -17,7 +17,6 @@ go_library(
     deps = [
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/core:go_default_library",
-        "//pkg/apis/extensions:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/kubectl:go_default_library",
         "//pkg/kubectl/cmd/templates:go_default_library",
@@ -69,15 +68,12 @@ go_test(
         "//pkg/api/testapi:go_default_library",
         "//pkg/api/testing:go_default_library",
         "//pkg/apis/core:go_default_library",
-        "//pkg/kubectl:go_default_library",
-        "//pkg/kubectl/genericclioptions:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",
     ],

pkg/kubectl/cmd/util/factory.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
@@ -76,13 +75,6 @@ type ClientAccessFactory interface {
 	// and which implements the common patterns for CLI interactions with generic resources.
 	NewBuilder() *resource.Builder
 
-	// MapBasedSelectorForObject returns the map-based selector associated with the provided object. If a
-	// new set-based selector is provided, an error is returned if the selector cannot be converted to a
-	// map-based selector
-	MapBasedSelectorForObject(object runtime.Object) (string, error)
-	// ProtocolsForObject returns the <port, protocol> mapping associated with the provided object
-	ProtocolsForObject(object runtime.Object) (map[string]string, error)
-
 	// SuggestedPodTemplateResources returns a list of resource types that declare a pod template
 	SuggestedPodTemplateResources() []schema.GroupResource
 
@@ -154,16 +146,6 @@ func makePortsString(ports []api.ServicePort, useNodePort bool) string {
 	return strings.Join(pieces, ",")
 }
 
-func getProtocols(spec api.PodSpec) map[string]string {
-	result := make(map[string]string)
-	for _, container := range spec.Containers {
-		for _, port := range container.Ports {
-			result[strconv.Itoa(int(port.ContainerPort))] = string(port.Protocol)
-		}
-	}
-	return result
-}
-
 // Extracts the protocols exposed by a service from the given service spec.
 func getServiceProtocols(spec api.ServiceSpec) map[string]string {
 	result := make(map[string]string)

pkg/kubectl/cmd/util/factory_client_access.go

@@ -40,8 +40,6 @@ import (
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
-	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
@@ -118,58 +116,6 @@ func (f *ring0Factory) RESTClient() (*restclient.RESTClient, error) {
 	return restclient.RESTClientFor(clientConfig)
 }
 
-func (f *ring0Factory) MapBasedSelectorForObject(object runtime.Object) (string, error) {
-	// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
-	switch t := object.(type) {
-	case *api.ReplicationController:
-		return kubectl.MakeLabels(t.Spec.Selector), nil
-	case *api.Pod:
-		if len(t.Labels) == 0 {
-			return "", fmt.Errorf("the pod has no labels and cannot be exposed")
-		}
-		return kubectl.MakeLabels(t.Labels), nil
-	case *api.Service:
-		if t.Spec.Selector == nil {
-			return "", fmt.Errorf("the service has no pod selector set")
-		}
-		return kubectl.MakeLabels(t.Spec.Selector), nil
-	case *extensions.Deployment:
-		// TODO(madhusudancs): Make this smarter by admitting MatchExpressions with Equals
-		// operator, DoubleEquals operator and In operator with only one element in the set.
-		if len(t.Spec.Selector.MatchExpressions) > 0 {
-			return "", fmt.Errorf("couldn't convert expressions - \"%+v\" to map-based selector format", t.Spec.Selector.MatchExpressions)
-		}
-		return kubectl.MakeLabels(t.Spec.Selector.MatchLabels), nil
-	case *extensions.ReplicaSet:
-		// TODO(madhusudancs): Make this smarter by admitting MatchExpressions with Equals
-		// operator, DoubleEquals operator and In operator with only one element in the set.
-		if len(t.Spec.Selector.MatchExpressions) > 0 {
-			return "", fmt.Errorf("couldn't convert expressions - \"%+v\" to map-based selector format", t.Spec.Selector.MatchExpressions)
-		}
-		return kubectl.MakeLabels(t.Spec.Selector.MatchLabels), nil
-	default:
-		return "", fmt.Errorf("cannot extract pod selector from %T", object)
-	}
-}
-
-func (f *ring0Factory) ProtocolsForObject(object runtime.Object) (map[string]string, error) {
-	// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
-	switch t := object.(type) {
-	case *api.ReplicationController:
-		return getProtocols(t.Spec.Template.Spec), nil
-	case *api.Pod:
-		return getProtocols(t.Spec), nil
-	case *api.Service:
-		return getServiceProtocols(t.Spec), nil
-	case *extensions.Deployment:
-		return getProtocols(t.Spec.Template.Spec), nil
-	case *extensions.ReplicaSet:
-		return getProtocols(t.Spec.Template.Spec), nil
-	default:
-		return nil, fmt.Errorf("cannot extract protocols from %T", object)
-	}
-}
-
 func (f *ring0Factory) SuggestedPodTemplateResources() []schema.GroupResource {
 	return []schema.GroupResource{
 		{Resource: "replicationcontroller"},

pkg/kubectl/cmd/util/factory_test.go

@@ -17,54 +17,11 @@ limitations under the License.
 package util
 
 import (
-	"strings"
 	"testing"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/sets"
 	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/kubectl"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 )
 
-func TestProtocolsForObject(t *testing.T) {
-	f := NewFactory(genericclioptions.NewTestConfigFlags())
-
-	pod := &api.Pod{
-		ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"},
-		Spec: api.PodSpec{
-			Containers: []api.Container{
-				{
-					Ports: []api.ContainerPort{
-						{
-							ContainerPort: 101,
-							Protocol:      api.ProtocolTCP,
-						},
-						{
-							ContainerPort: 102,
-							Protocol:      api.ProtocolUDP,
-						},
-					},
-				},
-			},
-		},
-	}
-
-	expected := sets.NewString("101/TCP", "102/UDP")
-	protocolsMap, err := f.ProtocolsForObject(pod)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-
-	protocolsString := kubectl.MakeProtocols(protocolsMap)
-	protocolsStrings := strings.Split(protocolsString, ",")
-	got := sets.NewString(protocolsStrings...)
-
-	if !expected.Equal(got) {
-		t.Fatalf("Protocols mismatch! Expected %v, got %v", expected, got)
-	}
-}
-
 func TestMakePortsString(t *testing.T) {
 	tests := []struct {
 		ports          []api.ServicePort

pkg/kubectl/polymorphichelpers/BUILD

@@ -10,9 +10,11 @@ go_library(
         "historyviewer.go",
         "interface.go",
         "logsforobject.go",
+        "mapbasedselectorforobject.go",
         "objectpauser.go",
         "objectresumer.go",
         "portsforobject.go",
+        "protocolsforobject.go",
         "rollbacker.go",
         "statusviewer.go",
         "updatepodspec.go",

pkg/kubectl/polymorphichelpers/interface.go

@@ -60,6 +60,20 @@ type UpdatePodSpecForObjectFunc func(obj runtime.Object, fn func(*v1.PodSpec) er
 // UpdatePodSpecForObjectFn gives a way to easily override the function for unit testing if needed
 var UpdatePodSpecForObjectFn UpdatePodSpecForObjectFunc = updatePodSpecForObject
 
+// MapBasedSelectorForObjectFunc will call the provided function on mapping the baesd selector for object,
+// return "" if object is not supported, or return an error.
+type MapBasedSelectorForObjectFunc func(object runtime.Object) (string, error)
+
+// MapBasedSelectorForObjectFn gives a way to easily override the function for unit testing if needed
+var MapBasedSelectorForObjectFn MapBasedSelectorForObjectFunc = mapBasedSelectorForObject
+
+// ProtocolsForObjectFunc will call the provided function on the protocols for the object,
+// return nil-map if no protocols for the object, or return an error.
+type ProtocolsForObjectFunc func(object runtime.Object) (map[string]string, error)
+
+// ProtocolsForObjectFn gives a way to easily override the function for unit testing if needed
+var ProtocolsForObjectFn ProtocolsForObjectFunc = protocolsForObject
+
 // PortsForObjectFunc returns the ports associated with the provided object
 type PortsForObjectFunc func(object runtime.Object) ([]string, error)
 

pkg/kubectl/polymorphichelpers/mapbasedselectorforobject.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package polymorphichelpers
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/kubectl"
+)
+
+// mapBasedSelectorForObject returns the map-based selector associated with the provided object. If a
+// new set-based selector is provided, an error is returned if the selector cannot be converted to a
+// map-based selector
+func mapBasedSelectorForObject(object runtime.Object) (string, error) {
+	// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
+	switch t := object.(type) {
+	case *api.ReplicationController:
+		return kubectl.MakeLabels(t.Spec.Selector), nil
+	case *api.Pod:
+		if len(t.Labels) == 0 {
+			return "", fmt.Errorf("the pod has no labels and cannot be exposed")
+		}
+		return kubectl.MakeLabels(t.Labels), nil
+	case *api.Service:
+		if t.Spec.Selector == nil {
+			return "", fmt.Errorf("the service has no pod selector set")
+		}
+		return kubectl.MakeLabels(t.Spec.Selector), nil
+	case *extensions.Deployment:
+		// TODO(madhusudancs): Make this smarter by admitting MatchExpressions with Equals
+		// operator, DoubleEquals operator and In operator with only one element in the set.
+		if len(t.Spec.Selector.MatchExpressions) > 0 {
+			return "", fmt.Errorf("couldn't convert expressions - \"%+v\" to map-based selector format", t.Spec.Selector.MatchExpressions)
+		}
+		return kubectl.MakeLabels(t.Spec.Selector.MatchLabels), nil
+	case *extensions.ReplicaSet:
+		// TODO(madhusudancs): Make this smarter by admitting MatchExpressions with Equals
+		// operator, DoubleEquals operator and In operator with only one element in the set.
+		if len(t.Spec.Selector.MatchExpressions) > 0 {
+			return "", fmt.Errorf("couldn't convert expressions - \"%+v\" to map-based selector format", t.Spec.Selector.MatchExpressions)
+		}
+		return kubectl.MakeLabels(t.Spec.Selector.MatchLabels), nil
+	default:
+		return "", fmt.Errorf("cannot extract pod selector from %T", object)
+	}
+}

pkg/kubectl/polymorphichelpers/protocolsforobject.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package polymorphichelpers
+
+import (
+	"fmt"
+	"strconv"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/extensions"
+)
+
+func protocolsForObject(object runtime.Object) (map[string]string, error) {
+	// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
+	switch t := object.(type) {
+	case *api.ReplicationController:
+		return getProtocols(t.Spec.Template.Spec), nil
+	case *api.Pod:
+		return getProtocols(t.Spec), nil
+	case *api.Service:
+		return getServiceProtocols(t.Spec), nil
+	case *extensions.Deployment:
+		return getProtocols(t.Spec.Template.Spec), nil
+	case *extensions.ReplicaSet:
+		return getProtocols(t.Spec.Template.Spec), nil
+	default:
+		return nil, fmt.Errorf("cannot extract protocols from %T", object)
+	}
+}
+
+func getProtocols(spec api.PodSpec) map[string]string {
+	result := make(map[string]string)
+	for _, container := range spec.Containers {
+		for _, port := range container.Ports {
+			result[strconv.Itoa(int(port.ContainerPort))] = string(port.Protocol)
+		}
+	}
+	return result
+}
+
+// Extracts the protocols exposed by a service from the given service spec.
+func getServiceProtocols(spec api.ServiceSpec) map[string]string {
+	result := make(map[string]string)
+	for _, servicePort := range spec.Ports {
+		result[strconv.Itoa(int(servicePort.Port))] = string(servicePort.Protocol)
+	}
+	return result
+}