diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index 3a006880caa..ea377b91d75 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -356,16 +355,12 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti return nil } - selector, err := labels.Parse(options.Selector) - if err != nil { - return err - } p := pruner{ mapper: mapper, clientFunc: f.UnstructuredClientForMapping, clientsetFunc: f.ClientSet, - selector: selector, + selector: options.Selector, visitedUids: visitedUids, cascade: options.Cascade, @@ -452,7 +447,7 @@ type pruner struct { clientsetFunc func() (internalclientset.Interface, error) visitedUids sets.String - selector labels.Selector + selector string cascade bool dryRun bool diff --git a/pkg/kubectl/cmd/top_node.go b/pkg/kubectl/cmd/top_node.go index f4609627fad..f535b734e97 100644 --- a/pkg/kubectl/cmd/top_node.go +++ b/pkg/kubectl/cmd/top_node.go @@ -132,23 +132,14 @@ func (o *TopNodeOptions) Validate() error { if len(o.ResourceName) > 0 && len(o.Selector) > 0 { return errors.New("only one of NAME or --selector can be provided") } - if len(o.Selector) > 0 { - _, err := labels.Parse(o.Selector) - if err != nil { - return err - } - } return nil } func (o TopNodeOptions) RunTopNode() error { var err error - selector := labels.Everything() + selector := labels.Everything().String() if len(o.Selector) > 0 { - selector, err = labels.Parse(o.Selector) - if err != nil { - return err - } + selector = o.Selector } metrics, err := o.Client.GetNodeMetrics(o.ResourceName, selector) if err != nil { @@ -167,7 +158,7 @@ func (o TopNodeOptions) RunTopNode() error { nodes = append(nodes, *node) } else { nodeList, err := o.NodeClient.Nodes().List(metav1.ListOptions{ - LabelSelector: selector.String(), + LabelSelector: selector, }) if err != nil { return err diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index eb25c5a00d1..7daa194f9b0 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -37,7 +37,6 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer/json" @@ -296,8 +295,8 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) Factory { // GetFirstPod returns a pod matching the namespace and label selector // and the number of all pods that match the label selector. -func GetFirstPod(client coreclient.PodsGetter, namespace string, selector labels.Selector, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*api.Pod, int, error) { - options := metav1.ListOptions{LabelSelector: selector.String()} +func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*api.Pod, int, error) { + options := metav1.ListOptions{LabelSelector: selector} podList, err := client.Pods(namespace).List(options) if err != nil { diff --git a/pkg/kubectl/cmd/util/factory_object_mapping.go b/pkg/kubectl/cmd/util/factory_object_mapping.go index 2b51808cf84..eae6e149177 100644 --- a/pkg/kubectl/cmd/util/factory_object_mapping.go +++ b/pkg/kubectl/cmd/util/factory_object_mapping.go @@ -296,7 +296,7 @@ func (f *ring1Factory) LogsForObject(object, options runtime.Object, timeout tim } sortBy := func(pods []*v1.Pod) sort.Interface { return controller.ByLogging(pods) } - pod, numPods, err := GetFirstPod(clientset.Core(), namespace, selector, timeout, sortBy) + pod, numPods, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy) if err != nil { return nil, err } @@ -405,7 +405,7 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object, timeout tim } sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) } - pod, _, err := GetFirstPod(clientset.Core(), namespace, selector, timeout, sortBy) + pod, _, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy) return pod, err } diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index fbd46b97561..d10897a1f37 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/pkg/kubectl/cmd/util/factory_test.go @@ -606,7 +606,7 @@ func TestGetFirstPod(t *testing.T) { } selector := labels.Set(labelSet).AsSelector() - pod, numPods, err := GetFirstPod(fake.Core(), metav1.NamespaceDefault, selector, 1*time.Minute, test.sortBy) + pod, numPods, err := GetFirstPod(fake.Core(), metav1.NamespaceDefault, selector.String(), 1*time.Minute, test.sortBy) pod.Spec.SecurityContext = nil if !test.expectedErr && err != nil { t.Errorf("%s: unexpected error: %v", test.name, err) diff --git a/pkg/kubectl/metricsutil/metrics_client.go b/pkg/kubectl/metricsutil/metrics_client.go index 77a65aa8209..fb0781c0f38 100644 --- a/pkg/kubectl/metricsutil/metrics_client.go +++ b/pkg/kubectl/metricsutil/metrics_client.go @@ -97,8 +97,8 @@ func nodeMetricsUrl(name string) (string, error) { return fmt.Sprintf("%s/nodes/%s", metricsRoot, name), nil } -func (cli *HeapsterMetricsClient) GetNodeMetrics(nodeName string, selector labels.Selector) ([]metricsapi.NodeMetrics, error) { - params := map[string]string{"labelSelector": selector.String()} +func (cli *HeapsterMetricsClient) GetNodeMetrics(nodeName string, selector string) ([]metricsapi.NodeMetrics, error) { + params := map[string]string{"labelSelector": selector} path, err := nodeMetricsUrl(nodeName) if err != nil { return []metricsapi.NodeMetrics{}, err diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 379d78accee..51d82a84b31 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -53,7 +53,7 @@ type Builder struct { stream bool dir bool - selector labels.Selector + selector *string selectAll bool includeUninitialized bool @@ -292,14 +292,10 @@ func (b *Builder) ResourceNames(resource string, names ...string) *Builder { // SelectorParam defines a selector that should be applied to the object types to load. // This will not affect files loaded from disk or URL. If the parameter is empty it is -// a no-op - to select all resources invoke `b.Selector(labels.Everything)`. +// a no-op - to select all resources invoke `b.Selector(labels.Everything.String)`. func (b *Builder) SelectorParam(s string) *Builder { - selector, err := labels.Parse(s) - if err != nil { - b.errs = append(b.errs, fmt.Errorf("the provided selector %q is not valid: %v", s, err)) - return b - } - if selector.Empty() { + selector := strings.TrimSpace(s) + if len(selector) == 0 { return b } if b.selectAll { @@ -310,8 +306,8 @@ func (b *Builder) SelectorParam(s string) *Builder { } // Selector accepts a selector directly, and if non nil will trigger a list action. -func (b *Builder) Selector(selector labels.Selector) *Builder { - b.selector = selector +func (b *Builder) Selector(selector string) *Builder { + b.selector = &selector return b } @@ -407,7 +403,8 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string case len(args) == 1: b.ResourceTypes(SplitResourceArgument(args[0])...) if b.selector == nil && allowEmptySelector { - b.selector = labels.Everything() + selector := labels.Everything().String() + b.selector = &selector } case len(args) == 0: default: @@ -595,7 +592,8 @@ func (b *Builder) visitorResult() *Result { } if b.selectAll { - b.selector = labels.Everything() + selector := labels.Everything().String() + b.selector = &selector } // visit items specified by paths @@ -655,7 +653,7 @@ func (b *Builder) visitBySelector() *Result { if mapping.Scope.Name() != meta.RESTScopeNameNamespace { selectorNamespace = "" } - visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, b.selector, b.export, b.includeUninitialized)) + visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, *b.selector, b.export, b.includeUninitialized)) } if b.continueOnError { result.visitor = EagerVisitorList(visitors) @@ -831,7 +829,11 @@ func (b *Builder) visitByPaths() *Result { visitors = NewDecoratedVisitor(visitors, RetrieveLatest) } if b.selector != nil { - visitors = NewFilteredVisitor(visitors, FilterBySelector(b.selector)) + selector, err := labels.Parse(*b.selector) + if err != nil { + return result.withError(fmt.Errorf("the provided selector %q is not valid: %v", b.selector, err)) + } + visitors = NewFilteredVisitor(visitors, FilterBySelector(selector)) } result.visitor = visitors result.sources = b.paths @@ -924,5 +926,5 @@ func MultipleTypesRequested(args []string) bool { } rKinds.Insert(rTuple.Resource) } - return (rKinds.Len() > 1) + return rKinds.Len() > 1 } diff --git a/pkg/kubectl/resource/helper.go b/pkg/kubectl/resource/helper.go index 940387295ad..7cb7bf4a39f 100644 --- a/pkg/kubectl/resource/helper.go +++ b/pkg/kubectl/resource/helper.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" @@ -65,12 +64,12 @@ func (m *Helper) Get(namespace, name string, export bool) (runtime.Object, error } // TODO: add field selector -func (m *Helper) List(namespace, apiVersion string, selector labels.Selector, export, includeUninitialized bool) (runtime.Object, error) { +func (m *Helper) List(namespace, apiVersion string, selector string, export, includeUninitialized bool) (runtime.Object, error) { req := m.RESTClient.Get(). NamespaceIfScoped(namespace, m.NamespaceScoped). Resource(m.Resource). VersionedParams(&metav1.ListOptions{ - LabelSelector: selector.String(), + LabelSelector: selector, }, metav1.ParameterCodec) if export { // TODO: I should be part of ListOptions @@ -82,14 +81,14 @@ func (m *Helper) List(namespace, apiVersion string, selector labels.Selector, ex return req.Do().Get() } -func (m *Helper) Watch(namespace, resourceVersion, apiVersion string, labelSelector labels.Selector) (watch.Interface, error) { +func (m *Helper) Watch(namespace, resourceVersion, apiVersion string, labelSelector string) (watch.Interface, error) { return m.RESTClient.Get(). NamespaceIfScoped(namespace, m.NamespaceScoped). Resource(m.Resource). VersionedParams(&metav1.ListOptions{ ResourceVersion: resourceVersion, Watch: true, - LabelSelector: labelSelector.String(), + LabelSelector: labelSelector, }, metav1.ParameterCodec). Watch() } diff --git a/pkg/kubectl/resource/helper_test.go b/pkg/kubectl/resource/helper_test.go index 00208bdf389..7ce9a3aad93 100644 --- a/pkg/kubectl/resource/helper_test.go +++ b/pkg/kubectl/resource/helper_test.go @@ -358,7 +358,7 @@ func TestHelperList(t *testing.T) { RESTClient: client, NamespaceScoped: true, } - obj, err := modifier.List("bar", api.Registry.GroupOrDie(api.GroupName).GroupVersion.String(), labels.SelectorFromSet(labels.Set{"foo": "baz"}), false, false) + obj, err := modifier.List("bar", api.Registry.GroupOrDie(api.GroupName).GroupVersion.String(), "foo=baz", false, false) if (err != nil) != test.Err { t.Errorf("unexpected error: %t %v", test.Err, err) } diff --git a/pkg/kubectl/resource/selector.go b/pkg/kubectl/resource/selector.go index 6faed0f4784..1afa4d1f7ba 100644 --- a/pkg/kubectl/resource/selector.go +++ b/pkg/kubectl/resource/selector.go @@ -21,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" ) @@ -30,13 +29,13 @@ type Selector struct { Client RESTClient Mapping *meta.RESTMapping Namespace string - Selector labels.Selector + Selector string Export bool IncludeUninitialized bool } // NewSelector creates a resource selector which hides details of getting items by their label selector. -func NewSelector(client RESTClient, mapping *meta.RESTMapping, namespace string, selector labels.Selector, export, includeUninitialized bool) *Selector { +func NewSelector(client RESTClient, mapping *meta.RESTMapping, namespace string, selector string, export, includeUninitialized bool) *Selector { return &Selector{ Client: client, Mapping: mapping, @@ -54,14 +53,14 @@ func (r *Selector) Visit(fn VisitorFunc) error { if errors.IsBadRequest(err) || errors.IsNotFound(err) { if se, ok := err.(*errors.StatusError); ok { // modify the message without hiding this is an API error - if r.Selector.Empty() { + if len(r.Selector) == 0 { se.ErrStatus.Message = fmt.Sprintf("Unable to list %q: %v", r.Mapping.Resource, se.ErrStatus.Message) } else { se.ErrStatus.Message = fmt.Sprintf("Unable to find %q that match the selector %q: %v", r.Mapping.Resource, r.Selector, se.ErrStatus.Message) } return se } - if r.Selector.Empty() { + if len(r.Selector) == 0 { return fmt.Errorf("Unable to list %q: %v", r.Mapping.Resource, err) } else { return fmt.Errorf("Unable to find %q that match the selector %q: %v", r.Mapping.Resource, r.Selector, err) diff --git a/test/e2e/kubectl/kubectl.go b/test/e2e/kubectl/kubectl.go index d84af0fe130..233707da358 100644 --- a/test/e2e/kubectl/kubectl.go +++ b/test/e2e/kubectl/kubectl.go @@ -554,7 +554,7 @@ var _ = SIGDescribe("Kubectl client", func() { ExecOrDie() Expect(runOutput).ToNot(ContainSubstring("stdin closed")) g := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) } - runTestPod, _, err := util.GetFirstPod(f.InternalClientset.Core(), ns, labels.SelectorFromSet(map[string]string{"run": "run-test-3"}), 1*time.Minute, g) + runTestPod, _, err := util.GetFirstPod(f.InternalClientset.Core(), ns, "run=run-test-3", 1*time.Minute, g) if err != nil { os.Exit(1) }