From 5d508760e39d7d5c05b5e066b848634db6b270e2 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 24 Jun 2019 09:34:15 -0700 Subject: [PATCH] Fix --watch-only of a single item with table output --- pkg/kubectl/cmd/get/BUILD | 3 +- pkg/kubectl/cmd/get/get.go | 62 +++++++++++++++-------------- pkg/kubectl/cmd/get/skip_printer.go | 48 ++++++++++++++++++++++ pkg/printers/tablegenerator.go | 9 +++-- pkg/printers/tableprinter.go | 10 +++-- 5 files changed, 95 insertions(+), 37 deletions(-) create mode 100644 pkg/kubectl/cmd/get/skip_printer.go diff --git a/pkg/kubectl/cmd/get/BUILD b/pkg/kubectl/cmd/get/BUILD index 58b82a922d0..c62cf65b3c6 100644 --- a/pkg/kubectl/cmd/get/BUILD +++ b/pkg/kubectl/cmd/get/BUILD @@ -22,6 +22,7 @@ go_library( "get.go", "get_flags.go", "humanreadable_flags.go", + "skip_printer.go", "sorter.go", "table_printer.go", ], @@ -60,6 +61,7 @@ go_library( "//vendor/github.com/spf13/cobra:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/integer:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", "//vendor/vbom.ml/util/sortorder:go_default_library", ], ) @@ -97,7 +99,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/streaming:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", diff --git a/pkg/kubectl/cmd/get/get.go b/pkg/kubectl/cmd/get/get.go index 5f39a5adbd4..d92937a0eff 100644 --- a/pkg/kubectl/cmd/get/get.go +++ b/pkg/kubectl/cmd/get/get.go @@ -49,12 +49,13 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/util/i18n" + utilpointer "k8s.io/utils/pointer" ) // GetOptions contains the input to the get command. type GetOptions struct { PrintFlags *PrintFlags - ToPrinter func(*meta.RESTMapping, bool, bool) (printers.ResourcePrinterFunc, error) + ToPrinter func(*meta.RESTMapping, *bool, bool, bool) (printers.ResourcePrinterFunc, error) IsHumanReadablePrinter bool PrintWithOpenAPICols bool @@ -230,7 +231,7 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri o.IsHumanReadablePrinter = true } - o.ToPrinter = func(mapping *meta.RESTMapping, withNamespace bool, withKind bool) (printers.ResourcePrinterFunc, error) { + o.ToPrinter = func(mapping *meta.RESTMapping, outputObjects *bool, withNamespace bool, withKind bool) (printers.ResourcePrinterFunc, error) { // make a new copy of current flags / opts before mutating printFlags := o.PrintFlags.Copy() @@ -257,6 +258,9 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri if o.Sort { printer = &SortingPrinter{Delegate: printer, SortField: sortBy} } + if outputObjects != nil { + printer = &skipPrinter{delegate: printer, output: outputObjects} + } if o.ServerPrint { printer = &TablePrinter{Delegate: printer} } @@ -539,7 +543,7 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e fmt.Fprintln(o.ErrOut) } - printer, err = o.ToPrinter(mapping, printWithNamespace, printWithKind) + printer, err = o.ToPrinter(mapping, nil, printWithNamespace, printWithKind) if err != nil { if !errs.Has(err.Error()) { errs.Insert(err.Error()) @@ -635,7 +639,8 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) info := infos[0] mapping := info.ResourceMapping() - printer, err := o.ToPrinter(mapping, o.AllNamespaces, false) + outputObjects := utilpointer.BoolPtr(!o.WatchOnly) + printer, err := o.ToPrinter(mapping, outputObjects, o.AllNamespaces, false) if err != nil { return err } @@ -664,25 +669,29 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) tableGK := metainternal.SchemeGroupVersion.WithKind("Table").GroupKind() // print the current object - if !o.WatchOnly { - var objsToPrint []runtime.Object - - if isList { - objsToPrint, _ = meta.ExtractList(obj) - } else { - objsToPrint = append(objsToPrint, obj) + var objsToPrint []runtime.Object + if isList { + objsToPrint, _ = meta.ExtractList(obj) + } else { + objsToPrint = append(objsToPrint, obj) + } + for _, objToPrint := range objsToPrint { + if o.IsHumanReadablePrinter && objToPrint.GetObjectKind().GroupVersionKind().GroupKind() != tableGK { + // printing anything other than tables always takes the internal version, but the watch event uses externals + internalGV := mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion() + objToPrint = attemptToConvertToInternal(objToPrint, legacyscheme.Scheme, internalGV) } - for _, objToPrint := range objsToPrint { - if o.IsHumanReadablePrinter && objToPrint.GetObjectKind().GroupVersionKind().GroupKind() != tableGK { - // printing anything other than tables always takes the internal version, but the watch event uses externals - internalGV := mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion() - objToPrint = attemptToConvertToInternal(objToPrint, legacyscheme.Scheme, internalGV) - } - if err := printer.PrintObj(objToPrint, writer); err != nil { - return fmt.Errorf("unable to output the provided object: %v", err) - } + if err := printer.PrintObj(objToPrint, writer); err != nil { + return fmt.Errorf("unable to output the provided object: %v", err) } - writer.Flush() + } + writer.Flush() + if isList { + // we can start outputting objects now, watches started from lists don't emit synthetic added events + *outputObjects = true + } else { + // suppress output, since watches started for individual items emit a synthetic ADDED event first + *outputObjects = false } // print watched changes @@ -691,18 +700,11 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) return err } - first := true ctx, cancel := context.WithCancel(context.Background()) defer cancel() intr := interrupt.New(nil, cancel) intr.Run(func() error { _, err := watchtools.UntilWithoutRetry(ctx, w, func(e watch.Event) (bool, error) { - if !isList && first { - // drop the initial watch event in the single resource case - first = false - return false, nil - } - // printing always takes the internal version, but the watch event uses externals // TODO fix printing to use server-side or be version agnostic objToPrint := e.Object @@ -714,6 +716,8 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) return false, err } writer.Flush() + // after processing at least one event, start outputting objects + *outputObjects = true return false, nil }) return err @@ -750,7 +754,7 @@ func (o *GetOptions) printGeneric(r *resource.Result) error { return utilerrors.Reduce(utilerrors.Flatten(utilerrors.NewAggregate(errs))) } - printer, err := o.ToPrinter(nil, false, false) + printer, err := o.ToPrinter(nil, nil, false, false) if err != nil { return err } diff --git a/pkg/kubectl/cmd/get/skip_printer.go b/pkg/kubectl/cmd/get/skip_printer.go new file mode 100644 index 00000000000..f02883cb1da --- /dev/null +++ b/pkg/kubectl/cmd/get/skip_printer.go @@ -0,0 +1,48 @@ +/* +Copyright 2019 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 get + +import ( + "io" + + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/cli-runtime/pkg/printers" +) + +// skipPrinter allows conditionally suppressing object output via the output field. +// table objects are suppressed by setting their Rows to nil (allowing column definitions to propagate to the delegate). +// non-table objects are suppressed by not calling the delegate at all. +type skipPrinter struct { + delegate printers.ResourcePrinter + output *bool +} + +func (p *skipPrinter) PrintObj(obj runtime.Object, writer io.Writer) error { + if *p.output { + return p.delegate.PrintObj(obj, writer) + } + + table, isTable := obj.(*metav1beta1.Table) + if !isTable { + return nil + } + + table = table.DeepCopy() + table.Rows = nil + return p.delegate.PrintObj(table, writer) +} diff --git a/pkg/printers/tablegenerator.go b/pkg/printers/tablegenerator.go index a9d62c45a61..d6bf97caf70 100644 --- a/pkg/printers/tablegenerator.go +++ b/pkg/printers/tablegenerator.go @@ -48,10 +48,11 @@ type handlerEntry struct { // will only be printed if the object type changes. This makes it useful for printing items // received from watches. type HumanReadablePrinter struct { - handlerMap map[reflect.Type]*handlerEntry - options PrintOptions - lastType interface{} - lastColumns []metav1beta1.TableColumnDefinition + handlerMap map[reflect.Type]*handlerEntry + options PrintOptions + lastType interface{} + lastColumns []metav1beta1.TableColumnDefinition + printedHeaders bool } var _ TableGenerator = &HumanReadablePrinter{} diff --git a/pkg/printers/tableprinter.go b/pkg/printers/tableprinter.go index eeea57762ba..2e0ec209193 100644 --- a/pkg/printers/tableprinter.go +++ b/pkg/printers/tableprinter.go @@ -80,18 +80,22 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er if table, ok := obj.(*metav1beta1.Table); ok { // Do not print headers if this table has no column definitions, or they are the same as the last ones we printed localOptions := h.options - if len(table.ColumnDefinitions) == 0 || reflect.DeepEqual(table.ColumnDefinitions, h.lastColumns) { + if h.printedHeaders && (len(table.ColumnDefinitions) == 0 || reflect.DeepEqual(table.ColumnDefinitions, h.lastColumns)) { localOptions.NoHeaders = true } if len(table.ColumnDefinitions) == 0 { // If this table has no column definitions, use the columns from the last table we printed for decoration and layout. // This is done when receiving tables in watch events to save bandwidth. - localOptions.NoHeaders = true table.ColumnDefinitions = h.lastColumns - } else { + } else if !reflect.DeepEqual(table.ColumnDefinitions, h.lastColumns) { // If this table has column definitions, remember them for future use. h.lastColumns = table.ColumnDefinitions + h.printedHeaders = false + } + + if len(table.Rows) > 0 { + h.printedHeaders = true } if err := decorateTable(table, localOptions); err != nil {