From 4e7c10a520c0c4f8feb8ca9185168e2496a2f56f Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 1 Mar 2017 18:33:08 -0500 Subject: [PATCH 1/2] Don't bypass filter on generic output It is inconsistent and confusing (filtering is orthogonal from output) and we don't want to regress behavior from 1.5. --- hack/lib/test.sh | 4 ++-- pkg/kubectl/cmd/get.go | 14 ++++---------- pkg/kubectl/cmd/get_test.go | 7 ++++--- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/hack/lib/test.sh b/hack/lib/test.sh index 1771a82160..901777876d 100644 --- a/hack/lib/test.sh +++ b/hack/lib/test.sh @@ -76,7 +76,7 @@ kube::test::object_assert() { local args=${5:-} for j in $(seq 1 ${tries}); do - res=$(eval kubectl get "${kube_flags[@]}" ${args} $object -o go-template=\"$request\") + res=$(eval kubectl get -a "${kube_flags[@]}" ${args} $object -o go-template=\"$request\") if [[ "$res" =~ ^$expected$ ]]; then echo -n ${green} echo "$(kube::test::get_caller 3): Successful get $object $request: $res" @@ -103,7 +103,7 @@ kube::test::get_object_jsonpath_assert() { local request=$2 local expected=$3 - res=$(eval kubectl get "${kube_flags[@]}" $object -o jsonpath=\"$request\") + res=$(eval kubectl get -a "${kube_flags[@]}" $object -o jsonpath=\"$request\") if [[ "$res" =~ ^$expected$ ]]; then echo -n ${green} diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index bde9402a39..6d8968aa8b 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -54,11 +54,12 @@ var ( ` + valid_resources + ` - This command will hide resources that have completed. For instance, pods that are in the Succeeded or Failed phases. - You can see the full results for any resource by providing the '--show-all' flag. + This command will hide resources that have completed, such as pods that are + in the Succeeded or Failed phases. You can see the full results for any + resource by providing the '--show-all' flag. By specifying the output as 'template' and providing a Go template as the value - of the --template flag, you can filter the attributes of the fetched resource(s).`) + of the --template flag, you can filter the attributes of the fetched resources.`) get_example = templates.Examples(` # List all pods in ps output format. @@ -184,13 +185,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ return cmdutil.UsageError(cmd, usageString) } - // always show resources when getting by name or filename, or if the output - // is machine-consumable, or if multiple resource kinds were requested. - 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() diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 3b15124ae8..cdd67fe6b6 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -229,10 +229,11 @@ func TestGetObjectsFiltered(t *testing.T) { {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{"pods"}, flags: map[string]string{"output": "yaml"}, resp: pods, expect: []runtime.Object{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": "true", "output": "yaml"}, resp: pods, expect: []runtime.Object{first, second}}, {args: []string{"pods"}, flags: map[string]string{"show-all": "false"}, resp: pods, expect: []runtime.Object{second}}, } @@ -731,8 +732,8 @@ func TestGetByFormatForcesFlag(t *testing.T) { cmd.Run(cmd, []string{"pods"}) showAllFlag, _ := cmd.Flags().GetBool("show-all") - if !showAllFlag { - t.Errorf("expected showAll to be true when getting resource by name") + if showAllFlag { + t.Errorf("expected showAll to not be true when getting resource") } } From 34e4337e574e6fe3caa461e36328921f5042f8f4 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 1 Mar 2017 18:34:47 -0500 Subject: [PATCH 2/2] Don't print the "filtered" message on generic output Unify the various output displays and make them simpler. Don't write to glog, but only output the info when `-v 2` to stderr. --- hack/make-rules/test-cmd-util.sh | 4 ++++ pkg/kubectl/cmd/get.go | 35 +++----------------------------- pkg/kubectl/cmd/util/helpers.go | 23 ++++++++++++++++++--- 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 54b5757bdd..da2fde186c 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -1106,6 +1106,10 @@ run_kubectl_get_tests() { # Post-condition: The text "No resources found" should be part of the output kube::test::if_has_string "${output_message}" 'No resources found' # Command + output_message=$(kubectl get pods --ignore-not-found 2>&1 "${kube_flags[@]}") + # Post-condition: The text "No resources found" should not be part of the output + kube::test::if_has_not_string "${output_message}" 'No resources found' + # Command output_message=$(kubectl get pods 2>&1 "${kube_flags[@]}" -o wide) # Post-condition: The text "No resources found" should be part of the output kube::test::if_has_string "${output_message}" 'No resources found' diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index 6d8968aa8b..42b3bcb3d5 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -243,13 +243,10 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ } // print the current object - filteredResourceCount := 0 if !isWatchOnly { if err := printer.PrintObj(obj, out); err != nil { return fmt.Errorf("unable to output the provided object: %v", err) } - filteredResourceCount++ - cmdutil.PrintFilterCount(filteredResourceCount, mapping.Resource, filterOpts) } // print watched changes @@ -259,7 +256,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ } first := true - filteredResourceCount = 0 intr := interrupt.New(nil, w.Stop) intr.Run(func() error { _, err := watch.Until(0, w, func(e watch.Event) (bool, error) { @@ -272,8 +268,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if err != nil { return false, err } - filteredResourceCount++ - cmdutil.PrintFilterCount(filteredResourceCount, mapping.Resource, filterOpts) return false, nil }) return err @@ -326,11 +320,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ return utilerrors.Reduce(utilerrors.Flatten(utilerrors.NewAggregate(errs))) } - res := "" - if len(infos) > 0 { - res = infos[0].ResourceMapping().Resource - } - var obj runtime.Object if !singleItemImplied || len(infos) > 1 { // we have more than one item, so coerce all items into a list @@ -351,7 +340,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ isList := meta.IsListType(obj) if isList { - filteredResourceCount, items, err := cmdutil.FilterResourceList(obj, filterFuncs, filterOpts) + _, items, err := cmdutil.FilterResourceList(obj, filterFuncs, filterOpts) if err != nil { return err } @@ -375,23 +364,17 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if err := printer.PrintObj(list, out); err != nil { errs = append(errs, err) } - - cmdutil.PrintFilterCount(filteredResourceCount, res, filterOpts) return utilerrors.Reduce(utilerrors.Flatten(utilerrors.NewAggregate(errs))) } - filteredResourceCount := 0 if isFiltered, err := filterFuncs.Filter(obj, filterOpts); !isFiltered { if err != nil { glog.V(2).Infof("Unable to filter resource: %v", err) } else if err := printer.PrintObj(obj, out); err != nil { errs = append(errs, err) } - } else if isFiltered { - filteredResourceCount++ } - cmdutil.PrintFilterCount(filteredResourceCount, res, filterOpts) return utilerrors.Reduce(utilerrors.Flatten(utilerrors.NewAggregate(errs))) } @@ -401,9 +384,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if err != nil { allErrs = append(allErrs, err) } - if len(infos) == 0 && len(allErrs) == 0 && !options.IgnoreNotFound { - outputEmptyListWarning(errOut) - } objs := make([]runtime.Object, len(infos)) for ix := range infos { @@ -426,12 +406,12 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ printer = nil var lastMapping *meta.RESTMapping w := printers.GetNewTabWriter(out) - filteredResourceCount := 0 if resource.MultipleTypesRequested(args) || cmdutil.MustPrintWithKinds(objs, infos, sorter) { showKind = true } + filteredResourceCount := 0 for ix := range objs { var mapping *meta.RESTMapping var original runtime.Object @@ -445,7 +425,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ if printer == nil || lastMapping == nil || mapping == nil || mapping.Resource != lastMapping.Resource { if printer != nil { w.Flush() - cmdutil.PrintFilterCount(filteredResourceCount, lastMapping.Resource, filterOpts) } printer, err = f.PrinterForMapping(cmd, mapping, allNamespaces) if err != nil { @@ -517,14 +496,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ } } w.Flush() - if printer != nil && lastMapping != nil { - cmdutil.PrintFilterCount(filteredResourceCount, lastMapping.Resource, filterOpts) - } + cmdutil.PrintFilterCount(errOut, len(objs), filteredResourceCount, len(allErrs), "", filterOpts, options.IgnoreNotFound) return utilerrors.NewAggregate(allErrs) } - -// outputEmptyListWarning outputs a warning indicating that no items are available to display -func outputEmptyListWarning(out io.Writer) error { - _, err := fmt.Fprintf(out, "%s\n", "No resources found.") - return err -} diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index b84612d06f..aafdcf7a46 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -698,9 +698,26 @@ func FilterResourceList(obj runtime.Object, filterFuncs kubectl.Filters, filterO return filterCount, list, nil } -func PrintFilterCount(hiddenObjNum int, resource string, options *printers.PrintOptions) { - if !options.NoHeaders && !options.ShowAll && hiddenObjNum > 0 { - glog.V(2).Infof(" info: %d completed object(s) was(were) not shown in %s list. Pass --show-all to see all objects.\n\n", hiddenObjNum, resource) +// PrintFilterCount displays informational messages based on the number of resources found, hidden, or +// config flags shown. +func PrintFilterCount(out io.Writer, found, hidden, errors int, resource string, options *printers.PrintOptions, ignoreNotFound bool) { + switch { + case errors > 0 || ignoreNotFound: + // print nothing + case found <= hidden: + if found == 0 { + fmt.Fprintln(out, "No resources found.") + } else { + fmt.Fprintln(out, "No resources found, use --show-all to see completed objects.") + } + case hidden > 0 && !options.ShowAll && !options.NoHeaders: + if glog.V(2) { + if hidden > 1 { + fmt.Fprintf(out, "info: %d objects not shown, use --show-all to see completed objects.\n", hidden) + } else { + fmt.Fprintf(out, "info: 1 object not shown, use --show-all to see completed objects.\n") + } + } } }