From cf86cb77eb90e35c91796172e24d7a8341691f80 Mon Sep 17 00:00:00 2001 From: Guoliang Wang Date: Wed, 23 May 2018 11:28:54 +0800 Subject: [PATCH] Move unrelated methods from the factory to helper --- pkg/kubectl/cmd/expose.go | 5 +- pkg/kubectl/cmd/util/BUILD | 4 -- pkg/kubectl/cmd/util/factory.go | 18 ------ pkg/kubectl/cmd/util/factory_client_access.go | 54 ---------------- pkg/kubectl/cmd/util/factory_test.go | 43 ------------- pkg/kubectl/polymorphichelpers/BUILD | 2 + pkg/kubectl/polymorphichelpers/interface.go | 14 +++++ .../mapbasedselectorforobject.go | 63 +++++++++++++++++++ .../polymorphichelpers/protocolsforobject.go | 63 +++++++++++++++++++ 9 files changed, 145 insertions(+), 121 deletions(-) create mode 100644 pkg/kubectl/polymorphichelpers/mapbasedselectorforobject.go create mode 100644 pkg/kubectl/polymorphichelpers/protocolsforobject.go diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index ba696073588..d36d47d736d 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/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 diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 4b83c6451f6..0fc82b7eaa2 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/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", ], diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 04e64f65ffa..b89cdf070c2 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/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 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) diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 0477aa45ad3..c31a1e107c8 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/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"}, diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index 910a8b2da67..06d9ac87062 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/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 diff --git a/pkg/kubectl/polymorphichelpers/BUILD b/pkg/kubectl/polymorphichelpers/BUILD index b50030a3c86..08132fceba2 100644 --- a/pkg/kubectl/polymorphichelpers/BUILD +++ b/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", diff --git a/pkg/kubectl/polymorphichelpers/interface.go b/pkg/kubectl/polymorphichelpers/interface.go index e2ecab3c20f..06f40df60be 100644 --- a/pkg/kubectl/polymorphichelpers/interface.go +++ b/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) diff --git a/pkg/kubectl/polymorphichelpers/mapbasedselectorforobject.go b/pkg/kubectl/polymorphichelpers/mapbasedselectorforobject.go new file mode 100644 index 00000000000..dd12941babc --- /dev/null +++ b/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) + } +} diff --git a/pkg/kubectl/polymorphichelpers/protocolsforobject.go b/pkg/kubectl/polymorphichelpers/protocolsforobject.go new file mode 100644 index 00000000000..de6d23d2a8e --- /dev/null +++ b/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 +}