diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index 21135c93501..bde9402a396 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -20,9 +20,9 @@ import ( "fmt" "io" + "github.com/golang/glog" "github.com/spf13/cobra" - "github.com/golang/glog" kapierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -163,9 +163,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if err != nil { return err } - filterFuncs := f.DefaultResourceFilterFunc() - filterOpts := f.DefaultResourceFilterOptions(cmd, allNamespaces) - cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err @@ -187,20 +184,18 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ return cmdutil.UsageError(cmd, usageString) } - argsHasNames, err := resource.HasNames(args) - if err != nil { - return err - } - // always show resources when getting by name or filename, or if the output // is machine-consumable, or if multiple resource kinds were requested. - if len(options.Filenames) > 0 || argsHasNames || cmdutil.OutputsRawFormat(cmd) { + if cmdutil.OutputsRawFormat(cmd) { if !cmd.Flag("show-all").Changed { cmd.Flag("show-all").Value.Set("true") } } export := cmdutil.GetFlagBool(cmd, "export") + filterFuncs := f.DefaultResourceFilterFunc() + filterOpts := f.DefaultResourceFilterOptions(cmd, allNamespaces) + // handle watch separately since we cannot watch multiple resource types isWatch, isWatchOnly := cmdutil.GetFlagBool(cmd, "watch"), cmdutil.GetFlagBool(cmd, "watch-only") if isWatch || isWatchOnly { @@ -224,6 +219,9 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if len(infos) != 1 { return i18n.Errorf("watch is only supported on individual resources and resource collections - %d resources were found", len(infos)) } + if r.TargetsSingleItems() { + filterFuncs = nil + } info := infos[0] mapping := info.ResourceMapping() printer, err := f.PrinterForMapping(cmd, mapping, allNamespaces) @@ -308,7 +306,9 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if err != nil { return err } - + if r.TargetsSingleItems() { + filterFuncs = nil + } if options.IgnoreNotFound { r.IgnoreErrors(kapierrors.IsNotFound) } diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index f8effc96a65..3b15124ae83 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -211,6 +211,59 @@ func TestGetObjects(t *testing.T) { } } +func TestGetObjectsFiltered(t *testing.T) { + initTestErrorHandler(t) + + pods, _, _ := testData() + pods.Items[0].Status.Phase = api.PodFailed + first := &pods.Items[0] + second := &pods.Items[1] + + testCases := []struct { + args []string + resp runtime.Object + flags map[string]string + expect []runtime.Object + }{ + {args: []string{"pods", "foo"}, resp: first, expect: []runtime.Object{first}}, + {args: []string{"pods", "foo"}, flags: map[string]string{"show-all": "false"}, resp: first, expect: []runtime.Object{first}}, + {args: []string{"pods"}, flags: map[string]string{"show-all": "true"}, resp: pods, expect: []runtime.Object{first, second}}, + {args: []string{"pods/foo"}, resp: first, expect: []runtime.Object{first}}, + {args: []string{"pods"}, flags: map[string]string{"output": "yaml"}, resp: pods, expect: []runtime.Object{first, second}}, + {args: []string{}, flags: map[string]string{"filename": "../../../examples/storage/cassandra/cassandra-controller.yaml"}, resp: pods, expect: []runtime.Object{first, second}}, + + {args: []string{"pods"}, resp: pods, expect: []runtime.Object{second}}, + {args: []string{"pods"}, flags: map[string]string{"show-all": "false"}, resp: pods, expect: []runtime.Object{second}}, + } + + for i, test := range testCases { + t.Logf("%d", i) + f, tf, codec, _ := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.UnstructuredClient = &fake.RESTClient{ + APIRegistry: api.Registry, + NegotiatedSerializer: unstructuredSerializer, + Resp: &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, test.resp)}, + } + tf.Namespace = "test" + buf := bytes.NewBuffer([]byte{}) + errBuf := bytes.NewBuffer([]byte{}) + + cmd := NewCmdGet(f, buf, errBuf) + cmd.SetOutput(buf) + for k, v := range test.flags { + cmd.Flags().Lookup(k).Value.Set(v) + } + cmd.Run(cmd, test.args) + + verifyObjects(t, test.expect, tf.Printer.(*testPrinter).Objects) + + if len(buf.String()) == 0 { + t.Errorf("%d: unexpected empty output", i) + } + } +} + func TestGetObjectIgnoreNotFound(t *testing.T) { initTestErrorHandler(t) @@ -313,7 +366,7 @@ func verifyObjects(t *testing.T, expected, actual []runtime.Object) { var err error if len(actual) != len(expected) { - t.Fatal(actual) + t.Fatalf("expected %d, got %d", len(expected), len(actual)) } for i, obj := range actual { switch obj.(type) { @@ -330,7 +383,7 @@ func verifyObjects(t *testing.T, expected, actual []runtime.Object) { t.Fatal(err) } if !apiequality.Semantic.DeepEqual(expected[i], actualObj) { - t.Errorf("unexpected object: \n%#v\n%#v", expected[i], actualObj) + t.Errorf("unexpected object: %d \n%#v\n%#v", i, expected[i], actualObj) } } } @@ -658,7 +711,7 @@ func TestGetMultipleTypeObjectsWithDirectReference(t *testing.T) { } } -func TestGetByNameForcesFlag(t *testing.T) { +func TestGetByFormatForcesFlag(t *testing.T) { pods, _, _ := testData() f, tf, codec, _ := cmdtesting.NewAPIFactory() @@ -674,7 +727,8 @@ func TestGetByNameForcesFlag(t *testing.T) { cmd := NewCmdGet(f, buf, errBuf) cmd.SetOutput(buf) - cmd.Run(cmd, []string{"pods", "foo"}) + cmd.Flags().Lookup("output").Value.Set("yaml") + cmd.Run(cmd, []string{"pods"}) showAllFlag, _ := cmd.Flags().GetBool("show-all") if !showAllFlag { diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 7d4dc811668..9cb9c0b5f1b 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -567,25 +567,31 @@ func (b *Builder) visitorResult() *Result { } func (b *Builder) visitBySelector() *Result { + result := &Result{ + targetsSingleItems: false, + } + if len(b.names) != 0 { - return &Result{err: fmt.Errorf("name cannot be provided when a selector is specified")} + return result.withError(fmt.Errorf("name cannot be provided when a selector is specified")) } if len(b.resourceTuples) != 0 { - return &Result{err: fmt.Errorf("selectors and the all flag cannot be used when passing resource/name arguments")} + return result.withError(fmt.Errorf("selectors and the all flag cannot be used when passing resource/name arguments")) } if len(b.resources) == 0 { - return &Result{err: fmt.Errorf("at least one resource must be specified to use a selector")} + return result.withError(fmt.Errorf("at least one resource must be specified to use a selector")) } mappings, err := b.resourceMappings() if err != nil { - return &Result{err: err} + result.err = err + return result } visitors := []Visitor{} for _, mapping := range mappings { client, err := b.mapper.ClientForMapping(mapping) if err != nil { - return &Result{err: err} + result.err = err + return result } selectorNamespace := b.namespace if mapping.Scope.Name() != meta.RESTScopeNameNamespace { @@ -594,9 +600,12 @@ func (b *Builder) visitBySelector() *Result { visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, b.selector, b.export)) } if b.continueOnError { - return &Result{visitor: EagerVisitorList(visitors), sources: visitors} + result.visitor = EagerVisitorList(visitors) + } else { + result.visitor = VisitorList(visitors) } - return &Result{visitor: VisitorList(visitors), sources: visitors} + result.sources = visitors + return result } func (b *Builder) visitByResource() *Result { @@ -607,14 +616,20 @@ func (b *Builder) visitByResource() *Result { isSingleItemImplied = len(b.resourceTuples) == 1 } + result := &Result{ + singleItemImplied: isSingleItemImplied, + targetsSingleItems: true, + } + if len(b.resources) != 0 { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("you may not specify individual resources and bulk resources in the same call")} + return result.withError(fmt.Errorf("you may not specify individual resources and bulk resources in the same call")) } // retrieve one client for each resource mappings, err := b.resourceTupleMappings() if err != nil { - return &Result{singleItemImplied: isSingleItemImplied, err: err} + result.err = err + return result } clients := make(map[string]RESTClient) for _, mapping := range mappings { @@ -624,7 +639,8 @@ func (b *Builder) visitByResource() *Result { } client, err := b.mapper.ClientForMapping(mapping) if err != nil { - return &Result{err: err} + result.err = err + return result } clients[s] = client } @@ -633,12 +649,12 @@ func (b *Builder) visitByResource() *Result { for _, tuple := range b.resourceTuples { mapping, ok := mappings[tuple.Resource] if !ok { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("resource %q is not recognized: %v", tuple.Resource, mappings)} + return result.withError(fmt.Errorf("resource %q is not recognized: %v", tuple.Resource, mappings)) } s := fmt.Sprintf("%s/%s", mapping.GroupVersionKind.GroupVersion().String(), mapping.Resource) client, ok := clients[s] if !ok { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("could not find a client for resource %q", tuple.Resource)} + return result.withError(fmt.Errorf("could not find a client for resource %q", tuple.Resource)) } selectorNamespace := b.namespace @@ -650,7 +666,7 @@ func (b *Builder) visitByResource() *Result { if b.allNamespace { errMsg = "a resource cannot be retrieved by name across all namespaces" } - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf(errMsg)} + return result.withError(fmt.Errorf(errMsg)) } } @@ -664,31 +680,38 @@ func (b *Builder) visitByResource() *Result { } else { visitors = VisitorList(items) } - return &Result{singleItemImplied: isSingleItemImplied, visitor: visitors, sources: items} + result.visitor = visitors + result.sources = items + return result } func (b *Builder) visitByName() *Result { - isSingleItemImplied := len(b.names) == 1 + result := &Result{ + singleItemImplied: len(b.names) == 1, + targetsSingleItems: true, + } if len(b.paths) != 0 { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify a resource by arguments as well")} + return result.withError(fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify a resource by arguments as well")) } if len(b.resources) == 0 { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("you must provide a resource and a resource name together")} + return result.withError(fmt.Errorf("you must provide a resource and a resource name together")) } if len(b.resources) > 1 { - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf("you must specify only one resource")} + return result.withError(fmt.Errorf("you must specify only one resource")) } mappings, err := b.resourceMappings() if err != nil { - return &Result{singleItemImplied: isSingleItemImplied, err: err} + result.err = err + return result } mapping := mappings[0] client, err := b.mapper.ClientForMapping(mapping) if err != nil { - return &Result{err: err} + result.err = err + return result } selectorNamespace := b.namespace @@ -700,7 +723,7 @@ func (b *Builder) visitByName() *Result { if b.allNamespace { errMsg = "a resource cannot be retrieved by name across all namespaces" } - return &Result{singleItemImplied: isSingleItemImplied, err: fmt.Errorf(errMsg)} + return result.withError(fmt.Errorf(errMsg)) } } @@ -709,19 +732,25 @@ func (b *Builder) visitByName() *Result { info := NewInfo(client, mapping, selectorNamespace, name, b.export) visitors = append(visitors, info) } - return &Result{singleItemImplied: isSingleItemImplied, visitor: VisitorList(visitors), sources: visitors} + result.visitor = VisitorList(visitors) + result.sources = visitors + return result } func (b *Builder) visitByPaths() *Result { - singleItemImplied := !b.dir && !b.stream && len(b.paths) == 1 + result := &Result{ + singleItemImplied: !b.dir && !b.stream && len(b.paths) == 1, + targetsSingleItems: true, + } + if len(b.resources) != 0 { - return &Result{singleItemImplied: singleItemImplied, err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify resource arguments as well")} + return result.withError(fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify resource arguments as well")) } if len(b.names) != 0 { - return &Result{err: fmt.Errorf("name cannot be provided when a path is specified")} + return result.withError(fmt.Errorf("name cannot be provided when a path is specified")) } if len(b.resourceTuples) != 0 { - return &Result{err: fmt.Errorf("resource/name arguments cannot be provided when a path is specified")} + return result.withError(fmt.Errorf("resource/name arguments cannot be provided when a path is specified")) } var visitors Visitor @@ -746,7 +775,9 @@ func (b *Builder) visitByPaths() *Result { if b.selector != nil { visitors = NewFilteredVisitor(visitors, FilterBySelector(b.selector)) } - return &Result{singleItemImplied: singleItemImplied, visitor: visitors, sources: b.paths} + result.visitor = visitors + result.sources = b.paths + return result } // Do returns a Result object with a Visitor for the resources identified by the Builder. diff --git a/pkg/kubectl/resource/result.go b/pkg/kubectl/resource/result.go index efef5c00bca..c77502bfec0 100644 --- a/pkg/kubectl/resource/result.go +++ b/pkg/kubectl/resource/result.go @@ -41,8 +41,9 @@ type Result struct { err error visitor Visitor - sources []Visitor - singleItemImplied bool + sources []Visitor + singleItemImplied bool + targetsSingleItems bool ignoreErrors []utilerrors.Matcher @@ -50,6 +51,19 @@ type Result struct { info []*Info } +// withError allows a fluent style for internal result code. +func (r *Result) withError(err error) *Result { + r.err = err + return r +} + +// TargetsSingleItems returns true if any of the builder arguments pointed +// to non-list calls (if the user explicitly asked for any object by name). +// This includes directories, streams, URLs, and resource name tuples. +func (r *Result) TargetsSingleItems() bool { + return r.targetsSingleItems +} + // IgnoreErrors will filter errors that occur when by visiting the result // (but not errors that occur by creating the result in the first place), // eliminating any that match fns. This is best used in combination with