From 71a13f7f4bdcf99733a7eed67800694e99278a0f Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 27 Jan 2016 14:14:27 -0500 Subject: [PATCH] ExtractList should handle v1.RawExtension correctly Also fixes CustomColumnPrinter to pass decoder in, and ensures a test case tests the combined path. --- hack/test-cmd.sh | 6 ++++++ pkg/api/meta/help.go | 13 ++++++++---- pkg/api/meta/help_test.go | 26 +++++++++++++++++++++++ pkg/kubectl/custom_column_printer.go | 16 ++++++++------ pkg/kubectl/custom_column_printer_test.go | 6 ++++-- pkg/kubectl/resource_printer.go | 4 ++-- 6 files changed, 56 insertions(+), 15 deletions(-) diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 61bdb3a2a6e..a3dab0a567f 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -848,6 +848,12 @@ __EOF__ # Post-condition: redis-master and redis-slave services are created kube::test::get_object_assert services "{{range.items}}{{$id_field}}:{{end}}" 'kubernetes:redis-master:redis-slave:' + ### Custom columns can be specified + # Pre-condition: generate output using custom columns + output_message=$(kubectl get services -o=custom-columns=NAME:.metadata.name,RSRC:.metadata.resourceVersion 2>&1 "${kube_flags[@]}") + # Post-condition: should contain name column + kube::test::if_has_string "${output_message}" 'redis-master' + ### Delete multiple services at once # Pre-condition: redis-master and redis-slave services exist kube::test::get_object_assert services "{{range.items}}{{$id_field}}:{{end}}" 'kubernetes:redis-master:redis-slave:' diff --git a/pkg/api/meta/help.go b/pkg/api/meta/help.go index 3812f240fb8..7d1570bc202 100644 --- a/pkg/api/meta/help.go +++ b/pkg/api/meta/help.go @@ -72,12 +72,17 @@ func ExtractList(obj runtime.Object) ([]runtime.Object, error) { for i := range list { raw := items.Index(i) switch item := raw.Interface().(type) { + case runtime.RawExtension: + switch { + case item.Object != nil: + list[i] = item.Object + case item.RawJSON != nil: + list[i] = &runtime.Unknown{RawJSON: item.RawJSON} + default: + list[i] = nil + } case runtime.Object: list[i] = item - case runtime.RawExtension: - list[i] = &runtime.Unknown{ - RawJSON: item.RawJSON, - } default: var found bool if list[i], found = raw.Addr().Interface().(runtime.Object); !found { diff --git a/pkg/api/meta/help_test.go b/pkg/api/meta/help_test.go index 23af10734cc..435784fdcf1 100644 --- a/pkg/api/meta/help_test.go +++ b/pkg/api/meta/help_test.go @@ -67,6 +67,28 @@ func TestExtractList(t *testing.T) { } } +func TestExtractListV1(t *testing.T) { + pl := &v1.PodList{ + Items: []v1.Pod{ + {ObjectMeta: v1.ObjectMeta{Name: "1"}}, + {ObjectMeta: v1.ObjectMeta{Name: "2"}}, + {ObjectMeta: v1.ObjectMeta{Name: "3"}}, + }, + } + list, err := meta.ExtractList(pl) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if e, a := len(list), len(pl.Items); e != a { + t.Fatalf("Expected %v, got %v", e, a) + } + for i := range list { + if e, a := list[i].(*v1.Pod).Name, pl.Items[i].Name; e != a { + t.Fatalf("Expected %v, got %v", e, a) + } + } +} + func TestExtractListGeneric(t *testing.T) { pl := &api.List{ Items: []runtime.Object{ @@ -94,6 +116,7 @@ func TestExtractListGenericV1(t *testing.T) { Items: []runtime.RawExtension{ {RawJSON: []byte("foo")}, {RawJSON: []byte("bar")}, + {Object: &v1.Pod{ObjectMeta: v1.ObjectMeta{Name: "other"}}}, }, } list, err := meta.ExtractList(pl) @@ -109,6 +132,9 @@ func TestExtractListGenericV1(t *testing.T) { if obj, ok := list[1].(*runtime.Unknown); !ok { t.Fatalf("Expected list[1] to be *runtime.Unknown, it is %#v", obj) } + if obj, ok := list[2].(*v1.Pod); !ok { + t.Fatalf("Expected list[2] to be *runtime.Unknown, it is %#v", obj) + } } type fakePtrInterfaceList struct { diff --git a/pkg/kubectl/custom_column_printer.go b/pkg/kubectl/custom_column_printer.go index 431a114c748..fb29a0a7ed2 100644 --- a/pkg/kubectl/custom_column_printer.go +++ b/pkg/kubectl/custom_column_printer.go @@ -73,7 +73,7 @@ func massageJSONPath(pathExpression string) (string, error) { // // NAME API_VERSION // foo bar -func NewCustomColumnsPrinterFromSpec(spec string) (*CustomColumnsPrinter, error) { +func NewCustomColumnsPrinterFromSpec(spec string, decoder runtime.Decoder) (*CustomColumnsPrinter, error) { if len(spec) == 0 { return nil, fmt.Errorf("custom-columns format specified but no custom columns given") } @@ -90,7 +90,7 @@ func NewCustomColumnsPrinterFromSpec(spec string) (*CustomColumnsPrinter, error) } columns[ix] = Column{Header: colSpec[0], FieldSpec: spec} } - return &CustomColumnsPrinter{Columns: columns}, nil + return &CustomColumnsPrinter{Columns: columns, Decoder: decoder}, nil } func splitOnWhitespace(line string) []string { @@ -108,7 +108,7 @@ func splitOnWhitespace(line string) []string { // For example the template below: // NAME API_VERSION // {metadata.name} {apiVersion} -func NewCustomColumnsPrinterFromTemplate(templateReader io.Reader) (*CustomColumnsPrinter, error) { +func NewCustomColumnsPrinterFromTemplate(templateReader io.Reader, decoder runtime.Decoder) (*CustomColumnsPrinter, error) { scanner := bufio.NewScanner(templateReader) if !scanner.Scan() { return nil, fmt.Errorf("invalid template, missing header line. Expected format is one line of space separated headers, one line of space separated column specs.") @@ -135,7 +135,7 @@ func NewCustomColumnsPrinterFromTemplate(templateReader io.Reader) (*CustomColum FieldSpec: spec, } } - return &CustomColumnsPrinter{Columns: columns}, nil + return &CustomColumnsPrinter{Columns: columns, Decoder: decoder}, nil } // Column represents a user specified column @@ -191,9 +191,11 @@ func (s *CustomColumnsPrinter) printOneObject(obj runtime.Object, parsers []*jso columns := make([]string, len(parsers)) switch u := obj.(type) { case *runtime.Unknown: - var err error - if obj, _, err = s.Decoder.Decode(u.RawJSON, nil, nil); err != nil { - return err + if len(u.RawJSON) > 0 { + var err error + if obj, err = runtime.Decode(s.Decoder, u.RawJSON); err != nil { + return fmt.Errorf("can't decode object for printing: %v (%s)", err, u.RawJSON) + } } } for ix := range parsers { diff --git a/pkg/kubectl/custom_column_printer_test.go b/pkg/kubectl/custom_column_printer_test.go index 41897f4b28f..531881a7327 100644 --- a/pkg/kubectl/custom_column_printer_test.go +++ b/pkg/kubectl/custom_column_printer_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/runtime" @@ -103,7 +104,7 @@ func TestNewColumnPrinterFromSpec(t *testing.T) { }, } for _, test := range tests { - printer, err := NewCustomColumnsPrinterFromSpec(test.spec) + printer, err := NewCustomColumnsPrinterFromSpec(test.spec, api.Codecs.UniversalDecoder()) if test.expectErr { if err == nil { t.Errorf("[%s] unexpected non-error", test.name) @@ -186,7 +187,7 @@ func TestNewColumnPrinterFromTemplate(t *testing.T) { } for _, test := range tests { reader := bytes.NewBufferString(test.spec) - printer, err := NewCustomColumnsPrinterFromTemplate(reader) + printer, err := NewCustomColumnsPrinterFromTemplate(reader, api.Codecs.UniversalDecoder()) if test.expectErr { if err == nil { t.Errorf("[%s] unexpected non-error", test.name) @@ -262,6 +263,7 @@ foo baz for _, test := range tests { printer := &CustomColumnsPrinter{ Columns: test.columns, + Decoder: api.Codecs.UniversalDecoder(), } buffer := &bytes.Buffer{} if err := printer.PrintObj(test.obj, buffer); err != nil { diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 974cff0dfc4..333aeb7a03a 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -113,7 +113,7 @@ func GetPrinter(format, formatArgument string) (ResourcePrinter, bool, error) { } case "custom-columns": var err error - if printer, err = NewCustomColumnsPrinterFromSpec(formatArgument); err != nil { + if printer, err = NewCustomColumnsPrinterFromSpec(formatArgument, api.Codecs.UniversalDecoder()); err != nil { return nil, false, err } case "custom-columns-file": @@ -121,7 +121,7 @@ func GetPrinter(format, formatArgument string) (ResourcePrinter, bool, error) { if err != nil { return nil, false, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) } - if printer, err = NewCustomColumnsPrinterFromTemplate(file); err != nil { + if printer, err = NewCustomColumnsPrinterFromTemplate(file, api.Codecs.UniversalDecoder()); err != nil { return nil, false, err } case "wide":